Skip to content

Prevent orphaned dev processes on shutdown - #2

Open
deverman wants to merge 1 commit into
loopwerk:developfrom
deverman:agent/prevent-orphaned-dev-processes
Open

Prevent orphaned dev processes on shutdown#2
deverman wants to merge 1 commit into
loopwerk:developfrom
deverman:agent/prevent-orphaned-dev-processes

Conversation

@deverman

Copy link
Copy Markdown

What changed

  • Run source recompiles and Ctrl-C shutdown on the same serial lifecycle queue.
  • Wait for the active site process to exit before stopping the server and exiting the CLI.

Why

The SIGUSR1 and SIGINT handlers previously ran on different queues while both accessed siteProcess. Ctrl-C during a source recompile could therefore terminate the old process and exit while the recompile handler installed a replacement, leaving the replacement orphaned and still watching the site.

Serializing those two lifecycle operations makes their ordering explicit. If shutdown arrives during a recompile, it runs immediately afterward and terminates the replacement before the CLI exits.

Validation

  • swift build
  • Ran saga dev against a fixture that deliberately delayed a source recompile, then sent Ctrl-C during that delay.
  • Verified the old and replacement child PIDs were both gone and the development-server port was closed after shutdown.

@deverman

Copy link
Copy Markdown
Author

I'm testing this locally a little more before I submit for review.

@deverman
deverman marked this pull request as ready for review August 1, 2026 05:22
@deverman

deverman commented Aug 1, 2026

Copy link
Copy Markdown
Author

@kevinrenskers I have been using this change for a few days and haven't found any issues. hope you will review and merge.

@kevinrenskers

Copy link
Copy Markdown
Member

Thanks for the PR! I only saw it now, for some reason GitHub didn't email me when you created it.

I'll review it this weekend.

@kevinrenskers

Copy link
Copy Markdown
Member

Thanks for this. The diagnosis is correct and the bug is real. The serialized queue is the right fix. However, Ctrl-C now pauses for the length of the rebuild, with no feedback.

recompileAndRelaunch() calls swiftBuild(), which runs swift build synchronously. That now occupies lifecycleQueue, so a SIGINT arriving mid-recompile queues behind it and the user sees no output at all, since print("\nShutting down...") is inside the blocked handler.

For a typical incremental rebuild that's a couple of seconds, so it's a papercut rather than a blocker. What makes it read as a hang rather than a delay is that it's unmaskable: the parent has SIGINT set to SIG_IGN, and the swift build child inherits that ignore across exec (it's spawned after line 121), so mashing Ctrl-C does nothing and the terminal can't interrupt the build either. It's also unbounded in the tail, since a Package.swift dependency change or a cold .build makes it noticeably longer.

At the very least we should move the shutdown message to something that isn't sitting behind the build. Even better would be for shutdown to not wait at all. Something like this?

// stored properties on DevCoordinator (lifecycleQueue is currently a local in start()):
private let lifecycleQueue = DispatchQueue(label: "Saga.Lifecycle")
private let shuttingDown = OSAllocatedUnfairLock(initialState: false)

// SIGINT source back on its own queue:
sigintSrc.setEventHandler { [weak self] in
  print("\nShutting down...")
  self?.shuttingDown.withLock { $0 = true }
  self?.lifecycleQueue.sync {}   // barrier: let any in-flight relaunch finish
  // ...terminate, stop, exit
}

// in recompileAndRelaunch, after swiftBuild() succeeds:
if shuttingDown.withLock({ $0 }) { return }

@kevinrenskers
kevinrenskers changed the base branch from main to develop August 1, 2026 09:52
@kevinrenskers

Copy link
Copy Markdown
Member

I have a slightly different implementation, which I'd love for you to have a look at: #3. Your commit is included in that PR, so you'll end up with the credit you're due.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants