PyMiniRacer, V8, and threading

[Ben Creech]

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?

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?

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?)

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.

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!).

(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

[Ben Noordhuis]

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.

> 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.

> 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.

[Ben Creech]

Much-belated thanks, Ben!

On Tue, Apr 9, 2024 at 4:50 AM Ben Noordhuis <in...@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-u...@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+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/v8-users/CAHQurc9EJhGevJRAU%2BaXzt%2BCQrrLAoCnHa0YLAagoiPHSf4Y8Q%40mail.gmail.com.