Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix process killing #1230

Closed
wants to merge 5 commits into from
Closed

Fix process killing #1230

wants to merge 5 commits into from

Conversation

Neptune650
Copy link
Contributor

/claim #1226
/closes #1226

@louis030195 Turns out in some places it was calling an non-existent function "kill_all_screenpipes", because the actual function was "kill_all_sreenpipes", renamed it and the calls, also uses a better method for terminating Windows processes properly.

Copy link

vercel bot commented Jan 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 28, 2025 10:05am

@louis030195
Copy link
Collaborator

Screenshot 2025-01-28 at 8 05 41 AM
  1. is false - no used code calls kill_all_screenpipes - this is dead code
  2. does it solve the problem? how can i know?

@Neptune650
Copy link
Contributor Author

Screenshot 2025-01-28 at 8 05 41 AM
  1. is false - no used code calls kill_all_screenpipes - this is dead code
  2. does it solve the problem? how can i know?

@louis030195 would a video suffice?

@louis030195
Copy link
Collaborator

i had some thoughts on this lately

i wonder if we can somehow "inject" something to tell pipes (bun, node) to kill itself when the API is down (screenpipe process not running) just like we do with the autodestruct feature of the CLI based on PID

not familiar how other software do it, there must be some best practice?

for example if you kill screenpipe with CLI or activity monitor you'll have bunch of bun hanging, or stuff like this, so the most reliable thing is to make the process itself check and stop

also i was wondering what is node doing here? why is node there if we use bun, isn't bun replacing node?

o3-mini answer

here's a deep dive:

process supervision fundamentals

when you design a system with child processes (like pipes written in bun/node) that rely on a parent (screenpipe) api being alive, you need a strategy to detect when that parent has died. common techniques include:

  • heartbeat or ping mechanism: the child periodically sends a heartbeat signal or ping to the parent. if the parent does not respond within a timeout window, the child elects to self-destruct.
  • parent process pid monitoring: in unix-like systems you can check if your parent's pid (or, more reliably, an OS-provided parent death signal mechanism like prctl with PR_SET_PDEATHSIG in linux) is still around. if the parent process is gone, the child can kill itself.
  • watchdog process: run a dedicated watchdog that monitors both parent and child processes and gracefully terminates the child if the parent isn’t available.

the core idea is that the child process “injects” a self-check. it determines whether it should continue running or self-terminate if it loses connectivity with the controlling process.

implementing the self-check in a bun/node context

for processes written in bun (or node) the approach typically involves periodic pings or even establishing a persistent communication channel (e.g., via sockets). if the channel drops or the ping fails, the pipe can decide to exit. for example, using a simple setInterval that checks the api's status can be effective. ensure that the check is robust; if you rely solely on a unidirectional signal (i.e., just sending a ping without handling network glitches) you risk false positives.

a more robust option is to implement a two-way handshake protocol (e.g., some message acknowledgement) so that the child is certain the api is down. this minimizes the risk of false shutdowns due to transient failures.

why node is still in the picture

bun aims to be a drop-in replacement for node. however:

  • compatibility concerns: existing projects or libraries might depend on node-specific behavior or modules that haven’t been ported to bun. so even if you develop in bun, certain parts—especially third-party tooling or plugins—might still invoke node.
  • ecosystem inertia: node has a massive ecosystem; bun, although promising, might not yet support every edge-case or specialized use case. developers might therefore still bundle node in their projects.
  • polyglot tooling: sometimes systems include both bun and node processes where one handles specific tasks better. for example, you might use bun for its speed and minimalism in certain parts of the app but still rely on node for tools where its maturity matters.

this mix is not uncommon in modern js environments. essentially, bun is replacing node gradually, but a full migration is rare because existing code and libraries may have critical dependencies on node’s runtime environment.

best practices for your context

given your pipeline (screenpipe api plus pipes in bun/node):

  • self-maintaining processes: implement a mechanism in your pipes that periodically verifies the api’s presence via a heartbeat or connection check. if the api is unresponsive, trigger a graceful shutdown.
  • logging & debugging: use lower case logging to track these events. for example, logging something like "api unreachable, terminating process" helps with debugging.
  • cross-platform considerations: make sure that your heartbeat or parent pid checking mechanism works consistently on macos, windows, and linux. each platform might have its own way to handle process pids and signals.
  • fail safe integration: if a pipe is orphaned (e.g., killed from the activity monitor), it should have a fallback timer after which it self-destructs if it can’t re-establish a connection.

summary

  1. use a periodic check (heartbeat or pid verification) in your bun pipes to detect the absence of the screenpipe api.
  2. design the self-check to avoid false positives by using two-way communication if possible.
  3. keep in mind that bun is still emerging; node may be used by legacy tooling or for compatibility with certain libraries, hence both might co-exist in your ecosystem.

although implementing these techniques might seem to complicate the architecture, it ultimately increases the robustness and reliability of your system—minimizing orphaned processes and ensuring a graceful shutdown when the primary control is lost.

#process-supervision #nodejs #bun #system-design

@louis030195 louis030195 closed this Feb 6, 2025
@Neptune650 Neptune650 deleted the kill_procs branch February 7, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bounty] properly end bun and node on windows when screenpipe close
2 participants