Add method to terminate a worker #420
Conversation
|
I imagine the CI will be able to run after merging #419 |
|
Not sure if I should bump the version within the PR or if that's done later? |
I've merged those changes. You can get the latest changes from master |
|
Awesome, thanks! I've updated the changelog, but I'm not sure if you wanted me to add a new |
There was a problem hiding this comment.
I am not sure if we should support termination.
Threads in Rust cannot be terminated at all without explicitly having a synchronous mechanism sending signals to them and they exit themselves.
A Worker is technically a thread and I feel it also should not support termination.
Terminating the worker would render the remaining bridges “poisoned”, however the current bridges simply drops any new messages and does not report the termination.
In addition, if ReactorBridge::next().await or OneshotBridge::run().await is called, they will be blocked forever.
|
Thanks for taking a look. You do have a good point regarding the no-terminaton-support of Rust threads, which I hadn't realized. However, after looking at some discussions relating to this (e.g. here) I don't think the same applies here. For threads, the concern is that if I e.g. lock a mutex then I can break invariants as long as I restore them before releasing the lock, but if I can be terminated then the lock will be released while some invariants are still broken. The Workers here don't have the same concurrency/shared memory issues: in this regard, they are much more like separate processes. So then the argument becomes more "that Workers should have a similar API" rather than "Rust threads don't have termination due to a key underlying reason, and so we avoid the feature for the same underlying reason". Regarding "poisoned" bridges: I think that a good solution would be to have Regarding |
Adds a
native_worker: RefCell<Option<DedicatedWorker>>field toWorkerBridgeInnerto allow immediately terminating a worker. Since there is no way to tell if a worker has been terminated from aweb_sys::Worker, uses the wrappingOptionto indicate this.Also removes some redundant
Rc. Now there are only twoRc-wrapped objects:WorkerBridgeInnerinstance shared between all forked bridges and (weakly) with the message receive handler.WorkerBridgeand (weakly) theWorkerBridgeInner.Fixes #408.