Conversation
`startUploadServer` checked only `authHeader.isEmpty`, though the comment directly above it said the uploader "needs a site root and an auth header". Android has checked both since it landed. An iOS host that configured an auth header but no `siteApiRoot` therefore started a server whose every request failed at the URLSession layer, instead of falling back to the WebView upload path the way Android does. `siteApiRoot` is a `URL` here where Android types it as a `String`, so `isEmpty()` has no direct equivalent — "addressable" is spelled as scheme and host both being present. Put the check in `MediaServerCredentials` rather than inline. `EditorViewController` is `#if canImport(UIKit)`, so it does not exist on the macOS host and nothing in it is reachable from the test suite — which is how the two platforms diverged here unnoticed. Outside the gate, the predicate gets five tests, including both arms of the site-root check.
The server reads the delegate three times per request — once at the admission gate (`handlesFile`), then again for `processFile` and `uploadFile` — and those reads are separated by a synchronous disk copy and an unbounded `processFile`. Held weakly, a host that released its delegate in that window changed the answer between reads: a file admitted for processing was forwarded to WordPress unprocessed. Hold it strongly, as Android already does with a plain `val`. Immutable strong references make the three reads agree by construction, and an in-flight upload keeps the delegate alive until it unwinds. The `weak` bought no leak protection to trade away. The cycle it named runs through `EditorViewController.mediaUploadDelegate` — a host object retaining the view controller forms `EditorViewController -> delegate -> EditorViewController` regardless of how this container holds it. What it did buy was the reference vanishing mid-request. So `mediaUploadDelegate` becomes strong too, and the machinery that existed only to police the old contract goes with it: `mediaUploadDelegateWasAssigned` and the released-before-load trap have nothing left to catch, because the editor now owns the delegate for its lifetime. Hosts no longer need to retain it themselves. `UploadContext` becomes a struct and drops its `@unchecked Sendable` opt-out: `MediaUploadDelegate` is `Sendable` and `DefaultMediaUploader` is `@unchecked Sendable`, so it is implicitly Sendable. `doesNotStronglyRetainDelegate` pinned the invariant being removed, so it is replaced by `retainsDelegateForServerLifetime`, asserting both halves — the server owns the delegate while it runs, and releases it afterward. `processesForHostReleasedDelegate` covers the bug directly; against a weak container it fails with the real symptom, `passthroughUploadCalled`. SwiftLint's `weak_delegate` is suppressed with the reasoning inline. The rule is arguably right that the name no longer fits — a later commit renames the property, and the suppression goes away with it.
`mediaUploadDelegate` is a strong `var`, which is only safe while the delegate does not retain the editor back. Assert that the editor still reaches `deinit` — and releases the delegate it owns — so a cycle introduced here fails a test instead of leaking silently. Extracted from the handler-ownership refactor that this branch drops: the server-side handler object lands further up the stack instead, but this half of the contract belongs with the change that creates it.
`NWListener.newConnectionHandler` retains the request handler and, through it, whatever the caller's closure captured — for the upload server, that is now the host's media delegate. `cancel()` does not drop the block: Network.framework holds the listener until cancellation completes on its own queue, so the final release landed there rather than on the thread that called `stop()`. A delegate reached through `EditorViewController.deinit` therefore deallocated off the main thread, measured at 46/50 on the listener's queue — `Timer.invalidate()` and `UIView` teardown in a host's `deinit` are both unsafe there. Clearing it after `cancel()` (not before — the listener is already torn down, so it is never live without a handler) makes teardown synchronous on the caller's thread. `retainsDelegateForServerLifetime` asserts the release outright instead of polling a one-second budget for it.
Silences the `#WeakMutability` warning this declaration emitted on every build — the only warning in the library and test targets.
Both comments claimed the retain cycle is one this code "can neither create nor prevent". Only the second half was true. Flipping `UploadContext` alone, with the property left `weak`, closes the ring through `uploadServer` and leaks the owner — that container's `weak` was its single weak link, so it demonstrably could prevent a cycle. The property is the same story in mirror image: strong here is exactly what lets a delegate that retains the editor back close the shorter ring, and `weak` would rule it out. Neither point argues against the change — the delegate vanishing mid-request is the failure that was actually being hit. But justifying it with a claim that does not hold is how the next investigation into a leaked editor gets misdirected.
`deinitReleasesEditorAndDelegate` passes unchanged against the pre-PR `weak` property, and passes with its `mediaUploadDelegate` assignment deleted outright. `LifetimeProbeDelegate` holds no reference to the editor, so the cycle the failure message names cannot be constructed in the fixture; and the test never touches `view`, so `viewDidLoad` never runs, `startUploadServer()` never runs, and the `UploadContext` this PR changes is never built. The ownership change is covered by `processesForHostReleasedDelegate` and `retainsDelegateForServerLifetime`, both of which fail against a weakly-held delegate with the real symptom. Covering the composite teardown path — editor loaded, server started, editor deallocated — needs a loaded editor and a real listener, which is E2E territory.
github-actions
Bot
added
the
[Type] Bug
jkmassel
added
[Type] Code Quality
XCFramework BuildThis PR's XCFramework is available for testing. Add the following to your .package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/669")Built from 0f5d2e2 |
The editor holds `mediaUploadDelegate` strongly so an in-flight upload can't
lose it mid-request — losing the delegate mid-request was the failure actually
being hit, and `weak` would rule it out. That is a deliberate trade, and this
commit pays the rest of its cost rather than leaving it in the docs.
A host object that owns the editor and is also its delegate closes a cycle ARC
cannot break, so `deinit` never runs and every editor opened strands a bound
loopback `NWListener` with a live token. `stopMediaHandling()` is the way out:
it stops the server, drops the delegate, and withdraws the endpoint from the
page. It has to be the host's call. Not because UIKit can't report a teardown
— an ancestor walk plus an orphan check at `viewDidDisappear` fires correctly
across fourteen hosting shapes, including this editor's shape in WordPress-iOS
— but because it can't report whether a detachment is *permanent*. A host may
re-present the same editor, and since the call is terminal, guessing wrong
disables media in an editor that survived.
Nothing here hands the delegate the editor: every value crossing that boundary
is a value type. So the cycle is entirely host-authored, and the rule is
narrower than "don't retain the editor" — don't conform the object that owns
it. That costs nothing, because `processFile` runs off the main actor and
could not have reached that object's state anyway.
The delegate also moves into `init` and becomes `public private(set)`. It only
takes effect if it is in place before the editor loads, a contract that used to
be enforced at runtime by a `precondition` on the setter; taking it at
construction makes that failure unrepresentable rather than caught, so
`hasStartedLoading` and the fail-fast go with it.
Stopping now also withdraws the endpoint from the page. The port and token are
injected once at document start, and `nativeMediaUploadMiddleware` refuses to
retry a failed native upload directly, on the stated assumption that an
advertised port is a reachable one ("cleared on stop"). Nothing cleared it. A
stopped server left every image insert failing with a connection error on a
working connection. `revokeNativeUploadEndpoint()` clears the live page, the
`localStorage` copy `getGBKit()` falls back to, and the injected user script
that would otherwise restore the dead port at the next document start.
A DEBUG-only census counts live upload servers and logs a fault past four.
Each live server is a bound loopback listener, one per editor, so monotone
growth is this cycle and nothing else produces it — `warmup()` passes no
delegate and starts no server. It is the only detectable symptom: a `deinit`
assertion cannot fire, because a cycle is what stops `deinit` from running.
Answers @dcalhoun's review question on this PR: ownership is the goal, and yes,
an explicit teardown was worth adding.
BREAKING CHANGE: `mediaUploadDelegate` is no longer settable; pass it to
`EditorViewController.init` instead.
jkmassel
force-pushed
the
jkmassel/silent-listener-failure
branch
from
September 15, 2026 22:39
41650f3 to
bfe5cf7
Compare
Both delivery paths could put bytes on the wire after the editor was gone. `stopMediaHandling()` and `EditorViewController.deinit` both call `stop()`, which cancels the in-flight connection tasks, but Swift cancellation is cooperative: the body read is an uninterruptible loop and a host's `processFile` need not check at all, so a request can reach delivery well after teardown. Whether it then actually reached WordPress rested entirely on URLSession noticing the cancellation. That is not a guarantee the server can rely on. `URLSessionProtocol` is public and documented for dependency injection, and the obvious conformance for a host wrapping a callback-based stack — `withCheckedThrowingContinuation` around a completion handler — has no cancellation awareness at all. Such a host would upload deterministically after teardown, and the response is discarded either way, leaving an attachment on the site that nothing cleans up. Check cancellation explicitly before delivery in `processAndUpload` and before the passthrough forward, so the guarantee comes from this file rather than from the HTTP client's behavior — and so `stopMediaHandling()`'s documented "any upload in flight is cancelled" holds for every host, not just those on a stock `URLSession`. `uploadErrorResponse` already logs CancellationError quietly, and HTTPServer drops the response for a cancelled task. Not covered by a test: reaching the window deterministically means driving teardown between the parse and the delivery of a live socket request, and a timing-based approximation would be flaky without pinning the behavior.
`retainsDelegateForServerLifetime` names two properties and only tested one. The delegate was bound to a strong local for the whole `do` block, so `#expect(weakDelegate != nil)` was satisfied by that local — the server's ownership was never what the assertion depended on. Confirmed by mutation. With `UploadContext(uploadDelegate: nil, ...)`, so the server holds no reference to the delegate at all, the test **passed**. Nil the host's reference before the assert — the way `processesForHostReleasedDelegate` already does — and the same mutation fails it. The release half was always live and is unchanged: no-op'ing `releaseConnectionHandler()` still fails the trailing `#expect(weakDelegate == nil)`, which is the regression 7124457 added it for. From 8827ba4, earlier in this branch — the commit that introduced the test to pin the strong-ownership fix it could not actually detect.
…legate The server holds the delegate strongly for the duration of a request, so a delegate that holds the server back closes a loop through the listener's captured blocks: `listener -> newConnectionHandler -> handler -> UploadContext -> delegate -> server`. Nothing else in the suite covers that edge — the editor-side tests never start a server, and `retainsDelegateForServerLifetime` uses a leaf delegate, so its release needs nothing to be broken first. Both assertions are load-bearing, confirmed by mutation. Building the context with `uploadDelegate: nil`, so the server holds no reference at all, fails the first: the delegate is freed as soon as the host's local goes out of scope. That stopping resolves the loop at all depends on Network.framework behaviour this package now relies on: for a deployment target of iOS 16 or later (this package requires 17), cancelling an NWListener releases the blocks it captured (rdar://89677097, documented in the macOS 13 release notes). No-op'ing `releaseConnectionHandler()` — the explicit clear added in 7124457 — still frees the delegate, one poll tick later, which is that behaviour doing the work. Before it, the blocks were held for the listener's lifetime. Pinned so a regression, or a lowered deployment target, fails loudly instead of quietly stranding listeners.
`releaseConnectionHandler()`'s doc claimed teardown releases the handler's captures on the caller's thread, unqualified. That holds for an idle server. A request in flight keeps its own copy of what the handler captured until the task unwinds, so a server stopped mid-request releases last on the task's executor no matter what the clear does — and when the editor is the delegate's only owner, that is where the host's `deinit` runs. Stated on each surface a host reads. The `GutenbergKitHTTP` doc scopes its claim to an idle server and drops the sentence naming GutenbergKit's upload server — the package doesn't otherwise know its consumers, and the preceding sentence already covers the effect. `mediaUploadDelegate` says the release can be late and off the main thread. `stopMediaHandling()` stops implying that "cancelled" means "stopped now": cancellation is cooperative, so a `processFile` that ignores it runs to completion and holds the delegate until it returns. Both points from @dcalhoun's review of this PR.
`revokeNativeUploadEndpoint()` passed `completionHandler: nil` and used `try?` on the configuration rebuild, so both of its steps could fail without saying so. Diagnostics, not a fix — the residual risk is narrow. The two plausible failures are self-healing: a terminated WebContent process reloads through `controllerWebContentProcessDidTerminate`, and a page that hasn't loaded yet has no endpoint to withdraw. Both land on the rebuilt user script, which advertises no port. What is left is a live page whose eval failed anyway, holding a port nothing is listening on until the next document start, and a rebuild that threw, leaving that next document start with no `window.GBKit` at all. Neither should be invisible, and the second is the more interesting of the two: the load path lets the same call throw and aborts, so a failure here is strictly quieter than the one the editor already refuses to ignore. It can't route through this file's `evaluate(_:isCritical:)` helper, which hands errors to `handleError` and presents a `UIAlertController`: this runs while the editor is going away.
`startUploadServer()` assigned `self.uploadServer` after awaiting the bind, so a `stopMediaHandling()` landing in that window was undone. The server was stored after the endpoint had already been withdrawn from the page, and because the context captured the delegate before the await, the cycle the call exists to open stayed closed — the property was nil, the server's reference was not. Small window, and nobody is in it today. Binding a loopback listener measures at or under a millisecond (`startAndStop` reports 0.001s); the five-second `defaultStartTimeout` is a ceiling for a listener that can't become ready, not a typical wait. `stopMediaHandling()` is new in this branch and has no callers, and WordPress-iOS sets no delegate at all. Reaching this needs a host that adopts the retaining shape the docs discourage and then tears the editor down inside that millisecond. Worth four lines anyway. `startUploadServer()` has exactly one call site and runs at most once per editor, so with the guard, "terminal" is a property of the code rather than of how fast a listener binds. Not covered by a test: suspending a real bind mid-flight is the only way into the window, and a timing-based approximation would pass whether or not it got there.
jkmassel
force-pushed
the
jkmassel/silent-listener-failure
branch
from
September 16, 2026 19:41
bfe5cf7 to
5a88648
Compare
jkmassel
added this pull request to stack #690
September 17, 2026 18:33
jkmassel
force-pushed
the
jkmassel/silent-listener-failure
branch
2 times, most recently
from
September 21, 2026 20:35
e88ce4f to
12c0ece
Compare
jkmassel
force-pushed
the
jkmassel/silent-listener-failure
branch
from
September 21, 2026 21:55
9cb0bba to
e61297c
Compare
jkmassel
force-pushed
the
jkmassel/silent-listener-failure
branch
from
September 25, 2026 21:37
eb7cbd7 to
5ee2ab8
Compare
This was referenced Sep 25, 2026
`DefaultMediaUploader` reads as an implementation of a host-facing protocol — the "default" one, as against a host's. It is not. It is GutenbergKit's own HTTP client for the configured site: it performs the uploads no host took over, and it relays every media delete, because the editor only ever asks to delete `/wp/v2/media/<id>` on the configured site. Rename it, and the `defaultUploader` parameters and properties that carry it, on both platforms. Sweep the prose and error strings that used the retired vocabulary too, including the `UploadContext` doc header and Android's three media-client messages. The host-facing docs still say "the default uploader" as a role: `InternalMediaClient` is internal on both platforms, so naming it in prose a host reads would be worse. On iOS this also narrows two signatures. `passthroughResponse` and `handleDelete` took the whole `UploadContext` and touched only the client. Pass it directly. On the delete path that is more than tidiness: a deletion always relays to the configured site, never to a delegate. That was a convention the signature let you break; now the type won't. The three functions that keep the context genuinely need every field. Android's server holds the client as a constructor property rather than threading a context, so it needs the rename only — and because its `handleDelete` is an instance method with the delegate in scope, the delete-path convention stays a convention there. The type-level guarantee is iOS-only.
* feat: add MediaUploader, for a host that owns the whole upload
Performing a media upload — and retrying it — should be a single,
all-or-nothing responsibility: either GutenbergKit performs the upload and
owns its retries, or the host does. Both go to the same configured site;
the only difference is who executes the requests.
`MediaUploadDelegate.uploadFile` doesn't offer that. A host performs the
`POST /wp/v2/media` and returns the raw response it received — then the
editor, reading that response, drives the `post-process` retries and the
orphan cleanup behind it, through the WebView rather than the host's
stack. A host that took over uploads to run them through its own
networking still didn't own the retries. It also receives no form fields,
so an attachment it uploads lands unattached to its post.
Add `MediaUploader`, which owns the upload end to end:
- `upload(_:)` returns the finished attachment or throws. There is no raw
response left for the editor to retry behind it, so the host drives its
own post-process recovery and force-deletes its own orphan on terminal
failure.
- It receives a `MediaUpload` carrying the file, its metadata, the
editor's non-file form fields (`post`, additionalData) and the request
query (`?_embed`) — everything needed to reproduce a native request.
- Fields are a `MediaUploadField` list rather than a dictionary, so
repeated names (a `field[]` array) survive verbatim and in order.
Additive for hosts: `uploadFile` still works and is marked deprecated,
pointing them at the replacement, and an uploader takes precedence when
both are set. Internally the upload server's startup gate widens to admit
an uploader as well as a delegate. GutenbergKit's own build keeps one
deprecation warning at the call site that supports the old hook — the
marker exists to tell hosts to migrate, and supporting the hook until it
is removed means calling it.
With an uploader set, the delegate's metadata gate can no longer decline a
file: the gate exists to skip a temp copy for a file the delegate won't
touch, but an uploader takes over delivery for *every* file, so passing
through would silently bypass it. Covered on both platforms.
`MediaUploadServerTest` crosses Detekt's LargeClass threshold; baselined
rather than split, which is its own change.
* fix: don't hand a declined file to processFile, and close the gaps around it
Follow-ups to the MediaUploader commit: a behavior bug in the metadata gate, a
cross-platform divergence, a missing cancellation check on Android, four
documentation defects, two test gaps, and a shadowed local.
- `processFile` ran on a file the delegate's metadata gate had declined. Widening
the gate to `uploader != nil || delegateWantsFile` left `processFile` called
unconditionally, so an image-only delegate paired with an uploader was handed
the `.mov` it had just said it won't touch — breaking the contract
`handlesFile` documents. `delegateWantsFile` is now carried into
`processAndUpload` and gates `processFile`. With an uploader set the file is
still delivered; it just skips processing on the way.
- iOS evaluated `handlesFile` eagerly while Android's `&&` short-circuited past
it, so the same host saw one callback per upload on iOS and zero on Android.
Android now binds it eagerly too: asked exactly once per upload on both.
- Android had no pre-flight cancellation check before handing work to the host
uploader, where iOS has `Task.checkCancellation()`. Added
`currentCoroutineContext().ensureActive()`, so a torn-down editor no longer
starts an upload whose attachment nobody would clean up.
- The recovery recipe omitted `post-process`'s required `action` parameter. Core
registers `action` as required, so a host following the doc verbatim would 400
five times and then run the doc's *other* instruction —
`DELETE /wp/v2/media/<id>?force=true` — destroying an attachment
`wp_update_image_subsizes()` would have recovered.
- `mediaUploader`'s doc had been appended to `mediaUploadDelegate`'s `///` block,
merging the two: `mediaUploadDelegate` shipped with no documentation and
`mediaUploader` opened by describing a delegate. Confirmed with
`swiftc -emit-symbol-graph` (`mediaUploadDelegate => None`); both now bind
their own 11 lines.
- `formFields` and `deprecatedUploadFile` were inserted between
`attachmentId(fromPath:)`'s doc and its declaration — merging into it on iOS,
dropping it outright on Android — costing the "deliberately narrow, not a
general REST proxy" rationale. Moved below their only caller, per AGENTS.md's
call-order rule.
- `ReplaceWith("MediaUploader")` takes a replacement *expression*; applying the
quick-fix drops all three arguments and leaves a type name where a
`MediaUploadResponse?` was expected. Removed, with a note so it doesn't return.
- Nothing pinned the Android gate or the uploader/deprecated-hook precedence:
deleting `&& mediaUploader == null` or reordering the two delivery paths left
the suite green. Three tests added; both mutations now fail.
- `DecliningDelegate` duplicated the pre-existing `DeclineByMetadataDelegate`
minus its `processFileCalled` recorder — the one probe that catches the
`processFile` bug above. Merged, and the declined-file test now asserts it.
- Two locals named `uploader` shadowed the new `MediaUploader` property, silently
(kotlinc has no diagnostic for it, detekt no rule). Renamed to `client`.
- `RecordingUploader`'s fixture carried no `title`, so the repo's only worked
example of an uploader result was a body that trips `transformAttachment`.
* feat!: remove MediaUploadDelegate.uploadFile `MediaUploader` replaces it. Returning a raw response split one upload's HTTP across two owners — the host performed the `POST`, the editor drove the `post-process` retries and orphan cleanup behind it — and the hook received no form fields, so an attachment it uploaded landed unattached to its post. Neither is fixable while the hook returns a raw response, which is what the replacement changes. What is left is a clean division: a delegate transforms bytes and GutenbergKit owns delivery and its retries; a `MediaUploader` owns delivery and its retries entirely. There is no longer an in-between where the host performs the upload but the editor retries it. `handlesFile` no longer gates the temp copy for two callers, only for `processFile` — and only when no uploader is set, since an uploader takes over delivery for every file. `MediaUploadResponse` drops to internal on both platforms: `uploadFile` was the only public API that named it. BREAKING CHANGE: hosts implementing `uploadFile` must conform to `MediaUploader` instead. Hosts that only implement `processFile` / `handlesFile` are unaffected. * docs: correct the handlesFile contract and the delegate's stale upload docs Review follow-ups to 6bcc210. No behavior change. `handlesFile`'s new doc said it is "only consulted when no `MediaUploader` is set". It is always consulted (`MediaUploadServer.swift:143`, `.kt:374`), and it still gates `processFile` (`.swift:306`, `.kt:534`) — a declined file reaches the uploader unprocessed. The implementation comment 160 lines away and the `an uploader sees a file the delegate's metadata gate would have declined` test on both platforms already said so. Replaced with wording lifted from that comment. Removing `uploadFile` also left the docs a host actually reads still advertising it: - The `mediaUploadDelegate` property summaries — what Xcode Quick Help and IDE hover show — said "customizing media file processing and upload behavior" (iOS) and "(resize, transcode, custom upload)" (Android). Both now describe transformation and point at `mediaUploader` for the upload case. - `MediaUploadResponse.statusCode` claimed the status could come from "the host's upload service". `MediaUploader.upload` returns `Data`, so the host path supplies a literal 201. - `MediaUploadServer`'s parameter docs, the `UploadResult.uploaded` doc, and Android's "won't process or upload" comment, whose iOS twin already read "won't process". Two non-doc changes ride along: - `UploadError.noUploader`'s message named a role the delegate no longer has: "No upload delegate or internal media client configured" becomes "No media uploader or ...". It reaches the editor in a 500 body; nothing asserts on it. - iOS's `MockUploadDelegate` became a duplicate of `ProcessOnlyDelegate` once `uploadFile` went. Android already consolidated on `ProcessOnlyDelegate`; iOS now matches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… class bound (#685) * refactor!: rename MediaUploadDelegate to MediaProcessor The protocol no longer uploads anything — the previous commit removed `uploadFile`, leaving `handlesFile` and `processFile`. "UploadDelegate" now describes the one thing it can't do, and next to `MediaUploader` the two names read as variations on the same job rather than the two halves of a deliberate split. `MediaProcessor` says what is left: it transforms bytes, GutenbergKit delivers them. Mechanical throughout — the property becomes `mediaProcessor`, the server parameter `processor`, the file `MediaHandlers.swift` (it holds both protocols now), and Android's demo `DemoMediaProcessor`. Prose follows the types. The `weak_delegate` suppression added when the property became strong goes away with the name: the rule was arguably right that a strongly-held "delegate" is a smell, and the answer was that this was never a delegate. BREAKING CHANGE: `mediaUploadDelegate` is now `mediaProcessor`, and `MediaUploadDelegate` is `MediaProcessor`. Conformances need no changes beyond the name. * refactor(ios)!: drop the class requirement from the media protocols `MediaProcessor` and `MediaUploader` were both `AnyObject`-bound, and `EditorViewController` holds both strongly. A conformer that holds the view controller back therefore closes a retain cycle ARC cannot break: the editor is never freed, so `deinit` never runs, so `uploadServer.stop()` — its only caller — never runs either, and a bound loopback `NWListener` outlives the editing session. Nothing needed class-boundness. There is no `weak`, `===`, or `ObjectIdentifier` use against either protocol anywhere in the tree, and every existing conformer is a class, which conforms unchanged. Dropping the requirement lets a host conform with a value type capturing only what the work needs — the shape that avoids the cycle, and the one a class-bound `Delegate` discouraged. This does not make the cycle impossible: a struct that stores the view controller cycles just the same. The docs say so rather than implying the type system settles it. * fix(ios): name every media handler in the leak census, not just the first `countServerStarted` resolved its name as `processor.map { … } ?? uploader.map { … }`, so with both supplied it always named the processor. The retainer is as likely to be the uploader — and after the class bound came off `MediaProcessor`, the processor it names may be a value type holding nothing at all, which is the one shape that provably cannot close the cycle the fault is reporting. A host following the docs hits this on the recommended shape: a leaf processor for the transform plus an uploader on the coordinator that owns the editor. The fault named the leaf, so the reader audits an object with no stored references, finds nothing, and concludes the census is broken. Names every handler that was supplied, and softens the assertion from "is its own media handler" to "is one of its own media handlers" — with two names it is a candidate list, not an accusation. DEBUG-only, and still behind the `count >= liveServerLeakThreshold` guard, so `String(describing: type(of:))` stays off the start path. * fix(ios): start the upload server for a host that supplies only an uploader `startUploadServer()` asks "did the host supply a media handler" twice — once before starting, once after the bind returns, because `stopMediaHandling()` can land while that `await` is suspended. The two reads had drifted. #628 widened the first to `delegate != nil || uploader != nil` and left the second checking the delegate alone. So a host that passed only a `mediaUploader` cleared the entry check, bound a loopback listener, then failed the post-bind check and stopped the server it had just started. `uploadServer` stayed nil, `buildEditorConfiguration` advertised `nativeUploadPort: nil`, and `api-fetch.js` fell through to the plain WebView path. The host's `upload(_:)` was **never called, for any file** — no error, no log. Uploads appeared to work; they just never reached the host's background session, offline queue, or retry policy, which is the whole reason to supply an uploader. Both reads now go through one `hasMediaHandling`, so they cannot disagree again. That is the actual defect — two hand-maintained copies of one predicate — and it is the same failure `MediaServerCredentials` was extracted for, where a check "diverged silently between iOS and Android once". `uploadServer` and `startUploadServer()` become internal so the suite can reach them. The test is parameterized over uploader-only, processor-only and both. Mutation-checked: restoring the old post-bind guard fails **only** the uploader-only case, which is the regression and nothing else. Android already pinned this gate (`GutenbergViewUploadServerTest`, "the upload server starts for an uploader with no delegate"); iOS had no equivalent, which is why the drift survived three commits green. * test: finish the rename in the media suites' own vocabulary The rename swept the helper types and left the names around them. 101 sites across four files: iOS test functions and `@Test` display strings, Kotlin backtick names, `let delegate = ProcessOnlyProcessor()` bindings that contradicted themselves on one line, `weakDelegate`, and a `// MARK: - Upload with delegate` header over code the production file had already renamed to `// MARK: - Processor Pipeline`. These are the strings CI prints. A red build named `retainsDelegateForServerLifetime` or `processes with the delegate, then delivers through the internal client` for a codebase where no symbol contains the word — Kotlin backticks are literally the JUnit report strings — so the first move on a failure was to grep for an API this stack deleted. Safe as a plain substring replacement: none of the four files reference a genuine delegate. `HttpServerDelegate` and `EditorViewControllerDelegate` live in other test files and are untouched. Two names would have read as stutters after a mechanical pass, so they say what the test does instead: `processesThenDelivers` and `processorRunsForUploader`. Test counts are unchanged — 590 iOS, and Android green on `--rerun-tasks` — so this renames tests rather than adding or dropping any. Note it does reset Buildkite Test Analytics history for the renamed cases, which is the deliberate cost. * test(ios): pin that a value type can conform to MediaProcessor Dropping `: AnyObject` is what the second commit here exists to deliver, and nothing exercised it — all eleven conformers in the tree were classes, so the boxed-existential path was never walked: copied into `UploadContext`, captured by the `@Sendable` handler closure, read again at `processFile`. Re-imposing the class bound, or breaking that path, would have compiled and passed green and surfaced only in a host's build. It now fails at compile time: error: non-class type 'ValueTypeProcessor' cannot conform to class protocol 'MediaProcessor' `ValueTypeProcessor` is `Sendable` without `@unchecked` — also the point, since that is the shape the protocol's documentation now recommends and the escape hatch it describes. The assertions run through `MockInternalMediaClient`'s recorded metadata rather than state on the processor, because a `struct` witnessing a non-mutating requirement cannot record anything. The transcoded mimeType and filename reaching the client could only come from `processFile` having actually run, so this pins invocation, not just storage.
…686) `MediaUploadServer` handled requests through static functions threading an `UploadContext` parameter through every call, because the closure form of `HTTPServer.start` can't capture the object that owns the server: the closure has to exist before the server does, and capturing `self` would form `MediaUploadServer -> HTTPServer -> handler -> MediaUploadServer`, so `deinit` — and its `stop()` — would never run. Add an `HTTPRequestHandler` protocol to `GutenbergKitHTTP` and a `start` overload that takes one, then serve `MediaUploadServer` from it. The dependencies become stored properties on a `Handler` struct and the request logic becomes instance methods. The closure overload is unchanged and forwards to the same code path, so the addition is purely additive — no existing caller, test, or the debug server is affected. Request handling is mandatory, so it can't be a defaulted `HTTPServerDelegate` method the way optional customization points are; hence an overload rather than a new delegate requirement. The protocol is deliberately not `AnyObject`-constrained so a handler *can* be a struct holding only what it needs — not because a struct is safe by construction. A value type is not protection: the server captures the handler into a heap node, so a struct storing the server's owner closes the same ring a class would. Both shapes work, under the same leaf discipline `HTTPServerDelegate` already documents — a handler must not strongly hold the object that owns the server. `Handler` stores no reference back to the `MediaUploadServer`, which is why the helpers outside it stay static. Mechanically: `handleRequest` becomes `handle`, the functions that use the dependencies become instance methods, and the ones that don't (`attachmentId`, `relayResponse`, `uploadErrorResponse`, `formFields`) stay static. `UploadContext` goes away — `Handler` is what it was. Helpers outside the handler (`errorResponse`, `writeStream`, `sanitizeFilename`, `uploadsTempDirectory`) are qualified rather than moved. No behavior change.
Setting a `mediaUploader` means the host is taking over uploads. With no site credentials the server would previously just not start, silently dropping the uploader — and its media deletes still need the internal media client to reach the configured site, since every attachment lives there no matter who delivered it. So the behavior forks by intent. A `mediaProcessor` with no credentials leaves the server down and uploads fall to the default WebView path — there is nothing to deliver through, so nothing to process. A `mediaUploader` with no credentials is a configuration error and fails fast: `precondition` on iOS, `check` on Android. The check runs where the host states its intent, not at page load. iOS takes its handlers at `init` and holds them `private(set)`, so a non-nil uploader at load time was necessarily passed at construction — checking there puts the caller's own line in the stack trace instead of surfacing the mistake from inside a page-load callback that names only GutenbergKit. Android still takes its handlers as mutable properties, so the earliest equivalent point is the `mediaUploader` setter, beside the existing set-before-load `check`. This is the shape `359d89ad` already established for the set-before-load contract: enforce the rule where the host states its intent. Android gains a `MediaServerCredentials` of its own, mirroring iOS's. The two predicates had diverged — iOS required an absolute site root while Android checked only `isEmpty()` — so `siteApiRoot = "example.com/wp-json/"`, which is what a user types when asked for their site address, trapped on iOS and started a doomed server on Android. Every relayed delete then threw `IllegalArgumentException` out of OkHttp's `.url()`, which is not an `IOException`, so it escaped `handleDelete` and degraded to a plain-text 500 the editor cannot parse — orphan cleanup failing silently. Both sides now test scheme and host. Emptiness is tested alongside nullity because `Uri` and `URL` disagree on a missing authority: `file:///tmp/wp-json` yields a null host on iOS and an empty one on Android. The policies live outside the view types on both platforms so they are reachable from the host test suites. On iOS that is load-bearing: `EditorViewController` is `#if canImport(UIKit)` and therefore absent from the macOS host, the one platform that can run Swift Testing's exit tests, so the trap itself is testable rather than only the predicate. The two suites assert matching cases on purpose — this policy has diverged silently once, and matching cases make the next divergence a failing test rather than a crash on one platform and a broken server on the other. A `mediaUploader` can still be dropped without failing, at the cleartext guard: an app that has not permitted cleartext to localhost never reaches the loopback server, so the uploader is never called. That one logs and degrades rather than failing, and the distinction is the cause rather than the symptom — missing credentials is an incoherent configuration, while blocked cleartext is a sound configuration the app's network policy blocks, and permitting cleartext makes the same setup work unchanged. Nothing had told integrators to permit it, so `docs/integration.md` now does, including why the library cannot ship the config itself: `networkSecurityConfig` is a single-valued `<application>` attribute, so a library declaring it fails the manifest merge against the host's and against other libraries that declare one — `rs.wordpress.api` already does.
`formFields` decodes every non-file form part as UTF-8. That can only be lossless because of who is on the other end, and nothing in the code enforces it -- so write it down on both platforms, in plain terms: only the editor's own page can reach the server, the browser guarantees form-field text is valid Unicode, and raw bytes always arrive carrying a filename, which routes them to the file rather than to a field. Pin the third condition with tests, since it is the one this code could break on its own. The bodies are ordered the way `uploadToServer` actually emits them -- file first, then additionalData -- and assert both the uploaded filename and the decoded fields, so the suite fails if either the partition or the file-selection rule changes. A second test covers the other half: valid UTF-8 round-trips, so real captions and titles are unaffected. The `buildMultipart` comments also claimed raw bytes were appended so a non-UTF-8 value would be "forwarded verbatim rather than coerced". That is not why -- such a value cannot reach them. On iOS they avoid a failable `String(data:encoding:)` whose `?? ""` would quietly drop a whole field; on both platforms they keep the re-encode byte-for-byte identical to the passthrough it replaces.
…ck (#689) `TranscodingProcessor` duplicated `ResizingProcessor` — same `.processed(_, mimeType: "video/mp4", filename: "clip.mp4")` result, one call site — and was the weaker of the two. It wrote to a fixed `$TMPDIR/clip.mp4` instead of a per-call UUID path inside the managed upload directory, and swallowed the write with `try?`, so a failed write still returned `.processed(<nonexistent URL>, …)` and the test passed green against a file that never existed. `ResizingProcessor` uses `try` and a unique path. Also drops `@unchecked Sendable` from `ThrowingUploader`, which has no stored properties and so satisfies `MediaUploader`'s inherited `Sendable` conformance on its own. The escape hatch is only needed by the mocks holding `NSLock`-guarded state; carrying it on a stateless one normalizes it as boilerplate, which is how an unsynchronized property gets added later without a diagnostic. `ContentTypeDeleteClient` keeps it — it subclasses `InternalMediaClient`, itself an `@unchecked Sendable` class, and must restate the conformance.
#651) `viewDidDisappear` cancelled `dependencyTaskHandle`, the async editor dependency fetch. That callback fires whenever the editor is merely covered — a full-screen modal presented over it, a push on top of it, a tab switch — and the fetch has exactly one starting point, the "no dependencies" branch of `viewDidLoad`, with nothing that restarts it. Cover a still-loading editor that way and the load is over for good: with the fetch parked mid-flight and `viewDidDisappear` delivered, the simulator shows the progress view replaced by the load-error screen and the host told `didFailToLoad` with a cancellation error. Coming back to the editor does nothing. The fast path a few lines above already carried the fix for this class of failure — the same cancellation landing mid `startUploadServer()` silently disabled native uploads for the session (#357) — but the async path never got the same treatment. Its task ends in the same `loadEditor()`, so that reason covers it too; its comment now says so, along with its own: nothing restarts the fetch. Stop cancelling rather than cancel-and-restart. A restart path would have to be idempotent and not race a fetch already in flight — complexity with nothing to buy. `deinit` is not an alternative home for the cancellation either, which is why `dependencyTaskHandle` goes away with the override rather than moving there. The task body is `await self?.prepareEditor()`, and optional- chaining a weak `self` into an async call holds a *strong* `self` across every suspension inside it, so the editor cannot be deallocated while the fetch is running. `deinit` is reachable only once the task has already finished, where there is nothing left to cancel. Not cancelling has a cost. The same retain keeps an editor released mid-fetch alive until the fetch and the load after it finish, which only URL timeouts bound. Meanwhile it keeps writing to the site's caches, and once the fetch lands it binds its upload server: a host that retains its own editor strands one more listener, and the DEBUG leak census can fire on a slow network. `[weak self]` still makes a task that has not started yet a no-op on an editor released first. Gating the cancellation on `isBeingDismissed`/`isMovingFromParent` was not an option. Hosts install this controller as a child, so UIKit sets those flags on an ancestor and they read `false` here — the gate would never fire, which is this change with a misleading condition on top. `EditorViewControllerLifecycleTests` pins both halves: covering the editor leaves the fetch running, and the fetch holds the editor alive until it finishes and releases it then. Against the old code the first fails with the real symptom, a cancelled request. The tests inject a `URLSessionProtocol` that holds every request until released, so the editor runs its real fetch path, and cover the editor through `beginAppearanceTransition`/`endAppearanceTransition` — `begin` alone never delivers `viewDidDisappear`. Each uses a fresh site host and deletes what it wrote, since `EditorViewController` can't be pointed at a temporary directory.
Base automatically changed from
jkmassel/dependency-fetch-cancelled
to
fix/own-media-delegate-strongly
October 1, 2026 22:43
…start Three related fixes to how `HTTPServer.start` observes and reports the listener's start, none of which changes a running server's behaviour. Breadcrumb after `.ready`. `start` nil'd `stateUpdateHandler` on `.ready` and nothing replaced it, so a listener that failed after a successful bind left nothing in GutenbergKit's own log — only Network.framework's `com.apple.network` line — while the server kept reporting a `port` and `token` addressing a socket nobody was listening on. Keep one handler for the listener's whole life instead: until `.ready` it feeds the start race; from `.ready` on it logs a `.failed` or `.waiting`. One handler rather than a replacement installed on `.ready`, because Network.framework binds each delivery to the handler installed when the delivery was queued — a replacement from the consumer, or even from the `.ready` callback on the listener's own queue, misses a state queued before it runs (confirmed on macOS 27 and the iOS 27 Simulator). It logs only to the unified log, which no host collects, so this makes the assumption falsifiable in a sysdiagnose rather than a fix for an observed failure; the failure that actually happens in the field is the socket reclamation handled by the upload-server restart later in this series, and that one delivers no state at all, so this can't see it. Log a pre-ready `.waiting`. `start` dropped a `.waiting` in `default: continue`, so a start that timed out threw a bare `startTimeout` and the log never said why. With the file-descriptor table full, a `127.0.0.1:0` listener goes straight to `.waiting(EMFILE)`; log the reason, and mark the adjacent `.failed` line `.public` too — default privacy redacts the reason to `<private>` on a device. Fix the DocC. `start(...)` documented a timeout as throwing `HTTPServerError/failedToStart`; it has thrown `startTimeout` since #561.
The editor's media upload server is a loopback `NWListener`. When the device becomes eligible for idle sleep — the ordinary case of a user locking their phone while the editor is open, unplugged, and left to idle — iOS reclaims the listening socket out from under the suspended app: every connection to the advertised port is then refused, while `NWListener` still reports `.ready` on the same port, so nothing is logged and the breadcrumb earlier in this PR never fires. Root cause and reproduction confirmed on an iPhone 15 Pro (iOS 27.0), a controlled A/B within one app session: - plugged in / kept awake, 27 minutes backgrounded -> socket still ALIVE - unplugged + locked + left to idle, ~6 minutes -> socket DEAD (connection refused), `NWListener` still `.ready` Battery was 100% both times, so the trigger is idle-sleep eligibility, not battery level or memory pressure. Traced from the iOS 27 kernelcache and RunningBoard: when nothing is left keeping the device awake, `runningboardd` (via `-[RBProcess _systemPreventIdleSleepStateDidChange:]`) calls `pid_shutdown_sockets` on suspended apps, which runs `networking_defunct_callout` -> `sosetdefunct`/`sodefunct` and tears down the listen socket in the kernel — with no state change delivered to the app, which is why nothing observes it. The editor kept handing the page that dead port, so every upload after the phone had been locked failed with a connection error until the editor was closed and reopened. So ask the port when the app returns to the foreground, and replace the server if it doesn't answer: - `MediaUploadServer.isAnswering()` sends an unauthenticated `GET /` over `NWConnection`. The server answers `407` and logs nothing, so a check that runs on every foreground stays silent, and no host's App Transport Security settings can decide the outcome. - `revokeNativeUploadEndpoint()` becomes `syncNativeUploadEndpoint()`. It already rewrote all three copies of the endpoint — the live page, the `localStorage` copy, and the injected user script — to withdraw it; now it writes the current port and token instead of always writing null. `nativeMediaUploadMiddleware` re-reads `window.GBKit` on every request, so the next upload picks up the new port and token with nothing to notify. - A restart that fails withdraws the endpoint, which is what that path did before: uploads fall back to the WebView's own rather than to a dead port. An upload already in flight when the socket is reclaimed still fails — the connection and its streamed body are gone, and `POST /wp/v2/media` is not idempotent, so it is deliberately not retried here — but the next upload the user retries lands on the restarted server. Only an editor with media handling has a server to restart, so a host that sets neither a processor nor an uploader is unaffected.
Adds "Upload Server Diagnostic" to the demo app's menu, to validate the loopback socket reclamation and its recovery on a device. - Live monitor: starts a loopback `HTTPServer` and, on every return to the foreground, checks whether the socket still answers. If iOS reclaimed it while the phone was locked/idle, it restarts the server and confirms it answers on the new port — the same check-and-restart the editor runs. - Recovery self-test: stops the server to stand in for the OS reclaiming the socket, confirms it stops answering, restarts it, and confirms it answers on the new port — proving the recovery path deterministically, without waiting to lock the phone. Uses the public `GutenbergKitHTTP.HTTPServer` (the type `MediaUploadServer` is built on) and the same `NWConnection` liveness probe as the shipped `isAnswering()`, so it exercises the identical socket behaviour and recovery. Demo app only; no library change.
An upload in flight when the user locks the phone stalls once the app is suspended, and dies if iOS then reclaims the app's sockets, which it does once the device can idle-sleep (see the restart commit). But a short upload usually only needs a few more seconds. Take a `UIApplication` background-task assertion for the duration of each upload, so locking the phone mid-transfer keeps the app running for the system's grace period (~30s) instead of suspending it immediately. A short upload then finishes and its socket survives the brief lock. The assertion is always balanced, including when the OS expires it first. Best-effort by design: the grace is fixed, so a long upload on a slow link still ends when it expires — the upload fails and the user retries, exactly as before. Real background continuation (surviving suspension or termination) belongs with a host `MediaUploader` over its own background `URLSession`, not the internal client. The demo app's Upload Server Diagnostic gains an "Active upload" toggle that holds the same assertion and shows the background time remaining, so the behaviour can be checked on a device: turn it on, lock the phone, and the socket stays alive for the grace period.
Several comments still said the system takes the upload server's listening socket when it suspends the app. Suspension alone doesn't: on an iPhone 15 Pro running iOS 27.0 the socket survived 27 minutes in the background while plugged in, and was gone after 6 minutes locked, unplugged, and left to idle. The sweep that reclaims it runs once the device becomes eligible for idle sleep, and only targets suspended apps. Reword those comments to match, and drop the "three seconds in the background was enough" figure from `isAnswering(timeout:)`, which implied a timing that suspension alone doesn't produce.
… probe `isAnswering()` treated only `.failed` and `.cancelled` as dead, but a refused TCP connection doesn't fail an `NWConnection`: it goes to `.waiting(ECONNREFUSED)` within a couple of milliseconds and stays there, waiting for a network path change that never comes on loopback. So the probe only reached "dead" when its own timeout cancelled the connection, and every recovery from a reclaimed socket waited out the full 2 seconds — the window in which the page still holds the dead port and an upload sent to it fails. Treat `.waiting` as dead too. A live loopback listener goes straight to `.ready`, and the existing check that a running server answers still passes. The new test stops a server, confirms with a plain BSD `connect()` that the port refuses, then times `isAnswering(timeout: 5s)`. Before this change it failed at 5.013s on macOS 27 and 5.092s on the iOS 27.0 Simulator; after it, it passes well under its 1s limit (worst of 25 runs: 0.076s). The demo app's Upload Server Diagnostic carries its own copy of the probe and had the same bug.
…vertises it Adds a Simulator test that runs in the editor's own `WKWebView`, from a `file://` page as the editor loads, and sends the request `nativeMediaUploadMiddleware` sends, built from `window.GBKit`: - live server → HTTP 201 - server stopped behind the editor's back → `TypeError: Load failed` in about 20ms, and nothing reaches the uploader - `restartUploadServerIfUnreachable()` → returns in under 1s (4.3ms here). Until it does, the page holds the dead port, so this is how long uploads fail after the app comes back. Without the `.waiting` probe fix it took 2.036s and fails. - after the restart → the page holds the new port and the upload lands (201) Removing `syncNativeUploadEndpoint()` from the restart path fails it: the page keeps the old port, and the upload after the restart is rejected too. The request mirrors the middleware's rather than running the bundled middleware, since no unit test loads the full editor. The middleware's part of the recovery is pinned in JS instead: - it reads the endpoint on every request, so a restarted server's port and token reach the next upload without re-registering anything. Capturing the endpoint on the first request fails this. - `getGBKit()` returns the live `window.GBKit`, so the `Object.assign` that `syncNativeUploadEndpoint()` runs is seen by the next read. Memoizing the first read fails this. - it neither retries a rejected `fetch` nor falls back to the WebView's own upload. That test's name promised no retry but only checked for no fallback, so it now also asserts `fetch` runs once.
The background-task assertion wrapped only the upload handler, which misses both ends of the exchange with the WebView. The file arrives before the handler runs, and `HTTPServer` writes WordPress's answer only after the handler returns. Locking the phone in the second gap can suspend the app after the attachment is created but before the editor hears about it. If iOS reclaims the socket before the app resumes, the editor shows a failure and a retry makes a duplicate. Add `HTTPServerDelegate.withConnectionActivity(_:)`, a scope the server runs each connection inside, from the first byte of the request until the response has been handed to the network stack. The default runs the connection unchanged. `MediaUploadServer`'s delegate holds the assertion there, so it now spans receiving the file, the upload, and writing the response, including the server's own error responses. The assertion must never wait for the main thread. Once it wraps every connection it also wraps the foreground liveness probe, and the main thread can be blocked for over 2s while a WebView's content process launches. A probe that waited for it would time out and restart a healthy server, cancelling any upload on it. So the assertion is now a `ProcessInfo` expiring activity rather than a `UIApplication` background task: it needs neither the main thread nor `UIApplication.shared`. On the iOS 27.0 Simulator, both kinds taken in the foreground kept the app running for the same ~26s after it was backgrounded. The demo's active-upload toggle now holds the same kind, so it still checks what ships. The assertion is no longer named after the file, because the connection opens before the request is parsed. That removes the `handleUpload`/`performUpload` split, which only existed to name it.
`restartUploadServerIfUnreachable()` probes the port and then acts on the server it probed, but the probe suspends, and another check can run in the meantime: two foregrounds in quick succession start one each. When both find the port dead, the first restarts the server, and the second then drops the server the first just started and starts another. Dropping it cancels any upload the page had already sent to it. After the probe, carry on only if the probed server is still the current one. The test holds each check's probe until it's told to answer, so it can resume the second check after the first has finished restarting. Without the guard, the late check replaces the fresh server. For this, `restartUploadServerIfUnreachable(isAnswering:)` takes the probe as a parameter, defaulting to `MediaUploadServer.isAnswering()`.
jkmassel
force-pushed
the
jkmassel/silent-listener-failure
branch
from
October 1, 2026 22:43
5ee2ab8 to
0f5d2e2
Compare
8 of 10 tasks
Contributor
Author
|
Superseded by #747. We're going with a different approach that doesn't need to manage sockets. #747 moves iOS media uploads off the loopback HTTP server and onto a #747 deletes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Four changes, all around the upload server's loopback socket (see Commits for how they split):
Root cause (found and reproduced)
The upload server is a loopback
NWListener. When the device becomes eligible for idle sleep — a user locking their phone while the editor is open, unplugged, and left to idle — iOS reclaims the listening socket out from under the suspended app. Connections to the advertised port are then refused, butNWListenerstill reports.readyon the same port, so nothing is logged and the editor comes back advertising a dead port.Device-confirmed on an iPhone 15 Pro (iOS 27.0), a controlled A/B in one app session:
NWListenerstill.readyBattery was 100% both times, so the trigger is idle-sleep eligibility, not battery level or memory pressure. Traced from the iOS 27 kernelcache + RunningBoard: when nothing keeps the device awake,
runningboardd(via-[RBProcess _systemPreventIdleSleepStateDidChange:]) callspid_shutdown_socketson suspended apps →networking_defunct_callout→sosetdefunct/sodefunct, tearing down the listen socket in the kernel with no state change delivered to the app — which is why nothing observes it.Fix
Ask the port when the app returns to the foreground; replace the server if it doesn't answer.
MediaUploadServer.isAnswering()sends an unauthenticatedGET /overNWConnection— the server answers407and logs nothing, so the check is silent and no host's ATS settings decide the outcome.NWConnectiondoesn't fail onECONNREFUSED— it sits in.waitingfor a path change that never comes — so the probe treats.waitingas dead instead of waiting out its 2s timeout while the page still holds the dead port.revokeNativeUploadEndpoint()becomessyncNativeUploadEndpoint(): it already rewrote all three copies of the endpoint (live page,localStorage, injected user script) to withdraw it; now it writes the current port and token.nativeMediaUploadMiddlewarere-readswindow.GBKitper request, so the next upload picks up the new endpoint with nothing to notify.Mid-flight uploads: the upload server holds a background-task assertion for each connection, from the first byte of the request until the response is handed back, so a short upload in flight when the phone is locked keeps running for the system's grace period (~30s) and finishes. It has to span the whole exchange, not just the upload handler: WordPress's answer is written after the handler returns, and an app suspended in that gap has created the attachment without the editor hearing about it, so a retry makes a duplicate. The assertion is a
ProcessInfoexpiring activity rather than aUIApplicationbackground task, so it never waits for the main thread — the liveness probe is served inside it, and a probe that waited out a busy main thread would restart a healthy server.A long upload that outlasts the grace still fails — the connection and its streamed body are gone, and
POST /wp/v2/mediaisn't idempotent, so it's deliberately not retried; the next upload the user retries lands on the restarted server. Continuation for long uploads (surviving suspension) belongs with a hostMediaUploaderover its own backgroundURLSession— a roadmap item, not this PR.Diagnostic (demo app)
⋯ menu → Upload Server Diagnostic. A live monitor starts a loopback
HTTPServerand, on every foreground, checks whether the socket still answers — restarting and confirming recovery if iOS reclaimed it. A recovery self-test stops the server (standing in for the OS), confirms it stops answering, restarts, and confirms it answers on the new port — proving recovery deterministically without locking the phone. An active-upload toggle holds the sameProcessInfoexpiring activity the upload server holds and shows the grace countdown, so locking the phone with it on demonstrates the upload surviving a brief lock. It uses the same publicHTTPServerandNWConnectionprobe as the shipped code.Testing
swift test— 590GutenbergKitTests+ 398GutenbergKitHTTPTestsxcodebuild test— 604 + 398, incl. new tests (a stopped server stops answering; an unreachable server is replaced; an answering one is left alone; a check that resumes after another restarted the server leaves the new one alone; a refused port is reported without waiting out the probe; an upload to a dead port fails until the restart re-advertises it; the connection activity spans the whole exchange; a connection is served while the main thread is busy)make lint-swiftcleanProcessInfoexpiring activity and aUIApplicationbackground task, both taken in the foreground, keep the app running for the same ~26s after it's backgrounded (iOS 27.0 Simulator)ProcessInfoexpiring activity, keeps the socket alive across a brief lockCommits
fix(ios): surface a listener failure that happens after a successful start— the breadcrumb, the pre-ready.waitinglog, and the DocC fix.fix(ios): restart the upload server when iOS reclaims its socket— the restart-on-foreground fix + tests.feat(ios): add an upload server diagnostic to the demo app— the on-device validation tool.fix(ios): hold a background-task assertion during a media upload— finish a short upload across a lock; extends the diagnostic.docs(ios): describe the idle-sleep socket sweep, not suspension— comments now match the root cause above.fix(ios): report a refused upload server port without waiting out the probe— treat.waiting(ECONNREFUSED)as dead, so recovery takes milliseconds instead of the probe's 2s timeout.test: pin that an upload to a dead port fails until the restart re-advertises it— the recovery, end to end in the editor's ownWKWebView.fix(ios): hold the upload assertion until the response is written— the assertion spans the whole connection via a newHTTPServerDelegatehook, and is aProcessInfoexpiring activity so it never waits for the main thread.fix(ios): restart the upload server once when foreground checks overlap— after the probe, only restart the server if it's still the one probed.Related