refactor(runtime): make the external event loop the only runner - #114
Merged
Conversation
… thread Preparation for driving Proton from an external event loop. Async's `ExternalEventLoop::poll` takes `timeout? : Int` where `None` means wait until something happens, and it wakes a blocked poll from a dedicated thread. Neither was expressible. `timeout_ms` becomes signed, with `PROTON_WAIT_TIMEOUT_INFINITE` (-1) the only negative value accepted. Signed matters beyond taste: a deadline computed as `deadline - now` goes negative once it has passed, and in an unsigned type that wraps to roughly 49 days -- a value indistinguishable from waiting forever, which hangs with no diagnostic. Under an unsigned timeout "wait forever" and "computed it wrong" are the same bit pattern. The sentinel does not escape C. `Runtime::wait` now takes `timeout_ms? : Int`, mirroring the trait exactly, and is the only place that encodes it. It clamps a negative `Some` to zero rather than failing, because arriving after a deadline is ordinary and is no reason to refuse to poll; the ABI stays strict so a foreign caller's arithmetic slip is still rejected. All three engines had assigned the timeout into an unsigned local, so each needed the infinite case spelled out. macOS gets an interval past any process lifetime, since CFRunLoopRunInMode has no forever. Linux passes -1 straight to poll(2), which uses the same convention, and now refuses an infinite wait when there is no wake descriptor rather than sleeping with nothing able to interrupt it. Windows would have produced the correct INFINITE by accident -- -1 in a DWORD is 0xFFFFFFFF -- which is the right answer for the wrong reason and would break the moment the sentinel changed, so it is written out. In every engine an engine deadline still overrides waiting forever: the wait exists to hand the loop back when there is work. `proton_runtime_signal_wakeup` is new and deliberately takes no handle. It is called from a thread that owns none, and handles validate thread ownership; it touches only atomics and the platform run loop. Every engine already ignored the runtime argument on this path, and the Windows wakeup source already handled NULL. Verified on macOS only, as agreed. Engine build and ctest 5/5, a second PROTON_WITH_ENGINE=OFF build covering the ABI layer and the stub engine 5/5, proton native 61/61 including a new test that an elapsed deadline polls rather than errors -- mutation-checked by removing the clamp, which fails it with the ABI's own message. `moon check --deny-warn` and `moon fmt --check` clean. The Linux and Windows engine edits are reviewed but compiled on neither.
The external-loop integration needs something for `poll` to block on from the moment async starts, which is before the application has decided what runtime to build -- it does file IO while reading its own manifest. Until now the only thing to block on was created during engine initialisation and belonged to a runtime, so a wakeup arriving in that window had nowhere to land and would have been dropped. The trait's contract makes a dropped wakeup a deadlock. `proton_host_loop_begin` / `wait` / `end` own that loop instead. It belongs to the main thread, not to a runtime: it starts first and outlives the last one. `wait` reports only wakeups until a runtime exists and pumps the engine afterwards. No second waiting implementation: `proton_engine_runtime_wait` now accepts a NULL runtime, and the host loop passes one. Every runtime helper it calls was already NULL-tolerant -- `has_bridge_request` checks explicitly, and the two that walk the window list filter on `window->runtime`, which matches nothing. Only the initialisation guard had to change. On macOS this required no new primitive at all. The wait source is plain CoreFoundation with no CEF dependency; it was merely called from engine init, so `host_loop_begin` calls it directly. It has to stay a CFRunLoopSource rather than a bare CFRunLoopWakeUp, because a source stays signalled until the loop next runs while a wakeup sent to an idle loop is lost. Windows maps just as directly: its pump event is already process-wide, so the host loop creates it, manual-reset so a wakeup with nothing waiting still releases the next wait. Linux refuses. Its wake pipe lives on the runtime struct (`runtime->wake_read_fd`), so there is nothing to block on during exactly the window the host loop exists to cover. Supporting it means hoisting that pipe to a process-wide one. Returning a loop that cannot be woken would deadlock, so `host_loop_begin` reports PROTON_ERR_UNSUPPORTED there instead -- loudly, and before anything depends on it. Verified on macOS: engine build and ctest 5/5, a PROTON_WITH_ENGINE=OFF build covering the ABI layer and the stub engine 5/5, and all three exports present. Windows and Linux are reviewed but compiled on neither.
The host loop exists to cover the window before the first runtime is created, but on Linux the only thing to block on lived on the runtime struct, so there was nothing to wait on during exactly that window. `host_loop_begin` refused rather than hand back a loop that could not be woken. The pipe is now process-wide, matching what the other two engines already had: macOS a run-loop source, Windows a pump event. That is also what the wakeup path always wanted -- `signal_wait_source` used to reach through `g_active_runtime` and silently drop the wakeup whenever no runtime was active, which is the same hole in a different place. Dropping it from the runtime struct removes the create and destroy plumbing that went with it: five `close_wake_pipe` calls on the various failure paths, and the field initialisation. As on macOS, `proton_engine_runtime_wait` now takes a NULL runtime and waits for host wakeups alone. Not compiled -- this is a Linux engine and the verification for it belongs to its owner. Reviewed for leftover `runtime->wake_*` references (none) and brace balance. macOS is unaffected and still builds with ctest 5/5.
`proton_host_loop_wait` blocked and reported what was ready, leaving the caller to pump the toolkit. That split works for a host that owns a runtime handle, because the pump is reachable as `proton_runtime_do_message_loop_work`. The host loop has no handle: it starts before the first runtime exists so that application code can do async work while it is still deciding what runtime to build. There was no way for it to reach the pump at all. So the wait becomes `proton_host_loop_poll`, one whole iteration of the loop: block, then advance the platform. That is also the honest mapping to the external-event-loop contract this exists to satisfy, which defines `poll` as "wait for events, process main loop specific events, return", not as a bare wait. Renamed rather than extended in place, because a `wait` that dispatches is a lie the next reader would have to discover. Blocking alone never dispatches on any of the three platforms. AppKit posts events to a run-loop source but only sends them from its own loop, which nobody is running; the same holds for GTK's pending sources and for the Win32 message queue. `cef_do_message_loop_work` has no other caller once the host loop owns the thread. A NULL runtime already meant "the whole process" for the wait itself. Two macOS helpers were still filtering windows by runtime pointer, which made that mean "no windows at all" and stranded browser creation for a runtime the host loop does not hold a handle to. Windows rejected a NULL runtime outright and then dereferenced it for the bridge handle, so the host loop could not have run there; it also gained the "host loop is not running" check the other two already had, and its manual-reset pump event is now cleared on every iteration rather than only once CEF exists -- a wakeup that arrived before the first runtime would otherwise have left every later wait returning immediately, forever. The stub engine now refuses `host_loop_begin` instead of accepting it. A loop that started there would fail on every poll instead, and a host that cannot get an engine should learn it while it is still the one raising the error.
AppKit insists on owning the process's main thread and `moonbitlang/async` wants a thread for its own loop. Proton's answer so far was `@proton.run`, which starts a pthread and moves all application code onto it. async's `set_external_event_loop` inverts that: MoonBit stays on the main thread and async moves only its waiting half elsewhere. `ProtonEventLoop` is the object that satisfies the contract, and `@proton.install_event_loop()` is how an application hands it over -- as the first statement of a plain `async fn main`, before any async work, because installing it after the loop has started aborts the process. `poll` maps to a single `host_loop_poll`, and notifies the runtime wake signal only when the returned mask has something in it. `poll` runs on every turn of async's loop, so notifying unconditionally would rouse the runtime pump on every timer tick anywhere in the application and make it re-poll native queues nothing has touched. The mask is trustworthy in the direction that matters: the platform bit is set whenever the loop did anything at all, so a wakeup cannot be dropped by gating on it. The wakeup callback is the one piece that runs on async's waiting thread, where the contract allows nothing but a direct C call with no refcounting. `@native.signal_wakeup` takes no handle for exactly that reason. The loop object is built at startup rather than on demand, because `poll` must not allocate a notification target the first time async calls it: the scheduler is already running by then, and a waiter that missed the object it was meant to wait on would never be woken. The timeout sentinel now has one encoder shared by `Runtime::wait` and `host_loop_poll` instead of a copy in each, keeping the promise that callers say `None` and never see `-1`. `@proton.run` and the wakeup pipe still exist; removing them is the next change.
The wait source (a CFRunLoopSource on macOS, a pump event on Windows) was created and destroyed by whatever CEF lifetime happened to be running. That was right when only a runtime ever waited. It is wrong now that a host loop owns it: the loop starts before the first runtime and has to outlive the last one, because the host keeps polling through its own shutdown -- and tearing the source down turned every one of those polls into "host loop is not running", which killed each e2e scenario app as it exited. So the source belongs to whoever created it. A host loop claims it in `host_loop_begin` and releases it in `host_loop_end`; a CEF lifetime that finds one already there leaves it alone, on both the create and the destroy side. macOS additionally stops rebuilding a source that already exists on this run loop, because rebuilding drops whatever was latched on it and a dropped wakeup deadlocks the host. Linux needed nothing: hoisting its wake pipe to process scope already left `close_wake_pipe` with a single caller, and its setup was already idempotent.
CEF asks to be pumped either now or in N milliseconds. Only the "now" case signalled the wait source; the delayed case signalled the wakeup fd, which no host reads any more. That was survivable while every host waited with a finite timeout, because it re-read the schedule on its way into the next wait. It is fatal to a host that waits indefinitely: the schedule is read once, before blocking, and one that arrives afterwards has nothing left to deliver it. The process then sits at 0% CPU with work pending, which is what hung every e2e scenario app. So both cases signal now. It does not spin, and the reason is the pump-active guard: the reschedule CEF issues while it is being pumped is suppressed, so after one early pump the loop settles onto the deadline rather than chasing the signal. Linux and Windows had no such guard -- nothing had needed one -- so they get the flag macOS already had. The mask is also no longer cleared on the way into a wait. Bits raised while the host was running its own code are the ones that matter most, and the pre-clear threw exactly those away; the exchange after the wait is what consumes them. Re-reporting a bit the host already handled costs it one spurious poll, while dropping one costs it the notification.
`managed_runtime_destroy_complete` was logged only by the managed app runner's destroy. Now that a host destroys its runtime inline, that marker is never written and the e2e lifecycle assertion had nothing to find, so three scenarios failed on a shutdown that had in fact completed. The inline path logs `runtime_destroy_complete` in the same place, and the assertion looks for that -- which the managed marker still contains, so it holds for both while both exist.
…c main Application code no longer lives on a thread Proton starts for it. An app is now a plain `async fn main` whose first statement installs Proton's event loop, and every line of MoonBit runs on the main thread that AppKit demanded in the first place. All 33 examples, both READMEs, and the scaffolding template say so. `RuntimeWakeup` loses its pipe. It existed to carry native notifications across a thread boundary that no longer exists; now the event loop's `poll` signals on the main thread and waiting is an ordinary scheduler wait. Its `revision`/`wait_after` surface is unchanged, so the session pump and the dialog pumps did not have to move. `RuntimeWakeupError` goes with it -- every one of its cases described a pipe or a wakeup-source feature that is gone. `@proton.run` stays, three lines long, for the one caller that cannot make `main` async: the e2e harness, which runs its scenario apps from inside a test. Its documentation says as much and points applications elsewhere. The pthread, the runner lifecycle states, and `app_thread_entry` are gone. `App::run` now refuses to start when no event loop is installed, in place of the old check for an active runner, and the message shows the new entry point. A scaffolded backend gains a direct `moonbitlang/async` dependency, because `async fn main` is only available to a package that imports it -- the compiler rejects the syntax outright otherwise, which is a confusing first error for a generated project.
DO NOT MERGE YET -- this deadlocks the e2e suite. See the note at the end. Asking every application to call `install_event_loop` as the first statement of `async fn main` puts a requirement in the language's blind spot: nothing enforces it, forgetting it fails much later at `App::run`, and the fix is a line the compiler will never suggest. `fn init` is where async's own documentation says to install an external loop, and it is the one place guaranteed to run before async starts its own, so Proton installs it there and an application is a plain `async fn main` again. The failure is stored rather than raised: `fn init` cannot raise, and aborting would kill a process that may never open a window. `App::run` raises it instead, where a caller can act on it. `install_event_loop` stops being public -- nothing outside needs it now -- and `@proton.run` is reduced to starting async on a loop that already exists. The facade suite passes, including a new test asserting that `fn init` actually ran, and every example and the scaffolding template lose the manual call. WHY THIS IS PARKED: `fn init` installs the loop in *every* process that links `@proton`, including the e2e orchestrator, which links it only for the scenario fixtures it runs in child processes. On Proton's loop that orchestrator deadlocks: its main thread sits in `host_loop_poll` with no timeout while its async waiting thread sits in `kevent`, and the scenario app it is driving idles in the same place. Reproduced twice; the same suite passes 1301/1302 on the commit before this one. It is most likely a lost wakeup that only shows under heavy IO, so it would bite a real application too and wants fixing on its own terms rather than by dropping this.
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.
Background
Proton had two competing application-runtime models:
moonbitlang/asyncintegration, but application code still entered through a native managed runner that moved MoonBit work to another thread;runtime_run/runtime_quitentry points alongside the external message pump.That split ownership forced each engine to maintain cross-thread dispatch, managed-shutdown coordination, duplicate wakeup lifetimes, and compatibility branches around CEF/AppKit/Win32/GTK operations. It also made it possible to select a runtime path that bypassed the external event loop and reintroduced the close, delayed-pump, and lost-wakeup failures this architecture is intended to prevent.
This PR makes the external event loop the only application runtime model. It intentionally removes the old APIs rather than retaining aliases or fallback behavior.
Changes
ExternalEventLoopduring facade initialization, beforemoonbitlang/asyncstarts scheduling.async fn mainprograms on the platform main thread; async owns waiting whileproton_host_loop_polladvances native platform and CEF work.@proton.run,@native.run_app,proton_app_run,proton_runtime_run, andproton_runtime_quit.managed_app_runnerfeature.Compatibility
This is a deliberate breaking cleanup. Applications should use an
async fn mainand callApp::run/run_or_abortdirectly. Low-level hosts must use the external-pump APIs; the blocking native run/quit route no longer exists.Validation
moon fmtmoon infomoon check --target native --deny-warnproton_smoketimeout)