Much-belated thanks, Ben! On Tue, Apr 9, 2024 at 4:50 AM Ben Noordhuis <i...@bnoordhuis.nl> wrote:
> On Tue, Apr 9, 2024 at 2:16 AM Ben Creech <b...@bpcreech.com> wrote: > > > > Hey folks, > > > > I've recently adopted the PyMiniRacer project; a minimalistic Python/V8 > bridge originally created by Sqreen. I revamped a lot of things, including > the threading model. I was curious if any experts out there want to take a > look at what I've done (it's all on the main branch, with the C++ code > here.) > > > > Some questions of varying levels of specificity: > > > > Overall threading design: > > I want to run the message loop indefinitely, processing potentially > delayed background work, and receiving new work from multiple (Python) > threads. It's particularly confusing how to reconcile the fact that I want > an always-running message pump loop (to, e.g., run time-delayed async work > like setTimeout callbacks) which AFAICT needs the lock, and run new > arbitrary tasks coming in from other threads (from Python). > > > > I didn't see an example of this; the two official examples don't show > how to run a program indefinitely (thus I'm using d8 as an example of > sorts). (Well, I tried to look at Chromium, but I got lost in the enormous > git download, heh.) So I sort of copied the model that the d8 tool seems to > use for its workers: I construct the Isolate in the same thread that runs > the message pump in a loop until shutdown. The message pump thread grabs > the Isolate lock and never gives it up. All interaction with the Isolate > then has to be done by either (A) posting tasks to the foreground runner or > (B) calling v8::Isolate::TerminateExecution. > > > > So the question is, uh, is that a reasonable model? > > Pinning isolates to threads? I'd say so. That's how worker_threads in > Node.js are implemented. > > > Thread safety of posting and terminating tasks: > > The above brings up a question: is it actually thread-safe to call > v8::Platform::GetForegroundTaskRunner(v8::Isolate *) and > v8::TaskRunner::PostTask(std::unique_ptr<v8::Task>) without the isolate > lock, as I'm currently assuming it is? It's not documented as safe! But it > seems to be safe upon cursory inspection of the implementation? > > Yes, I believe it is. > Thanks! I wonder if there's any way to get this documented (e.g., so it doesn't change on us later). > Does v8::platform::PumpMessageLoop need, get, or release the Isolate lock? > > Another question from the above: does v8::platform::PumpMessageLoop need > the Isolate lock? Does it ever unlock the Isolate lock? I.e., if you grab > the Isolate lock and then call it with MessageLoopBehavior::kWaitForWork, > it seems to sit around with the lock forever. > > > > I think the answers are: yes the message pump needs the lock, and no it > never unlocks it... thus my tentative conclusion is that if you're running > the message pump in an indefinite loop in blocking mode, you can't interact > with your Isolate on another thread. (Thus the task-posting solution: if > you want to interact with your Isolate which has its own infinite message > loop, just post a task to the task runner and it will run on the message > loop. Makes sense! But it's not written down anywhere AFAICT?) > > Technically the answer is probably "PumpMessageLoop doesn't need the > lock/is agnostic" but in practice it does because the tasks it > executes do. > Yeah I guess the issue is that I have no idea what tasks PumpMessageLoop might execute and whether they actually grab the Isolate lock. I can control for those tasks I put on the queue, but internal tasks like garbage collection I'm not sure about, and there doesn't seem to be any documentation about it. > > Is it safe to delete a v8::Persistent without the Isolate lock? > > I'm currently doing some awkward things to transport v8::Persistent > deletions onto the Isolate task queue, under the guess that it's not safe > to delete things without the lock. > > No, that's not safe. > > > Is it safe to decref a std::shared_ptr<v8::BackingStore> to 0, thus > deleting the BackingStore, without the Isolate lock? > > Same as above (but it's even weirder if dropping a std::shared_ptr > reference might be thread-unsafe!). > > I'm 95% sure the answer is that, yes, it's safe provided your > v8::ArrayBuffer::Allocator implementation is also thread-safe. > > > (End of questions.) > > > > Any insight on any of these things would be useful for me, and users of > the PyMiniRacer project! It seems to hang together fine under tests, but > thread safety is a curious art! > > > > And, while we're here, any other commentary on PyMiniRacer would be very > welcome. :) > > > > Thanks, > > Ben Creech > > -- > -- > v8-users mailing list > v8-users@googlegroups.com > http://groups.google.com/group/v8-users > --- > You received this message because you are subscribed to the Google Groups > "v8-users" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to v8-users+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/v8-users/CAHQurc9EJhGevJRAU%2BaXzt%2BCQrrLAoCnHa0YLAagoiPHSf4Y8Q%40mail.gmail.com > . > -- -- v8-users mailing list v8-users@googlegroups.com http://groups.google.com/group/v8-users --- You received this message because you are subscribed to the Google Groups "v8-users" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-users/CALZwtratLZcmWRYdDAQN7g7awgkX3VOaxc-5nP9q_CNdzi3JhA%40mail.gmail.com.
