Issue on V8 main branch with threads

[Meir Shpilraien]

Hey,

We recently encounter an issue in which our code, when run on V8 main branch, crashes with this fatal error:
#
# Fatal error in ../../src/execution/isolate.cc, line 4996
# Check failed: current_thread_id_ == thread_id (<unprintable> vs. <unprintable>).
#
#
#

I saw that this check was added recently on this PR: https://chromium-review.googlesource.com/c/v8/v8/+/5173257

Its purpose is to verify a correct use of entering and existing an isolate. But I do not see it is taking into consideration the Locker/Unlocker API. In case the current thread uses Unlocker and then another thread tries to enter the isolate it will failed.

Is my analysis correct or am I missing something?

Thanks,

Meir

[Nikos Papaspyrou]

Hi Meir,

You are correct about the purpose of the CHECK that I introduced in CL 5173257.

The documentation of the Locker/Unlocker API specifies that a thread has to exit the isolate before using v8::Unlocker and enter it again afterwards.

Without seeing your code, I cannot be sure that this is the issue but from what you write I suspect that it is. It should also be very easy to fix.

See here for a similar fix that had to be made in chromium: CL 5168625.

Best regards,

Nikos.

[Meir Shpilraien]

Thanks Nikos,

You are right, I missed it in our documentation. Will fix our code.

Thank you for the quick response.

[Meir Shpilraien]

Nikos I am still confuse.

The documentation also state that reenter the isolate is allowed: https://source.chromium.org/chromium/chromium/src/+/main:v8/include/v8-isolate.h;l=731

So before creating an unlocker we must exit the isolate the number of times we entered it (and then re-enter it again the same number of times after the unlocker is destroyed)?

Or is there some mechanism that handles it when we exit/enter the isolate?

[Nikos Papaspyrou]

The isolate can indeed be re-entered and, in that case, a counter is increased.

The isolate is only exited when the counter becomes zero.

This is indeed a different behaviour from what the documentation specifies for the Locker/Unlocker.

Do you have a scenario where it is not possible or convenient to properly exit the isolate (using the right number of Exit calls) before using the Unlocker?

If thread A has entered the isolate (N times) and then uses the Unlocker without properly exiting (N times), then it can still use the isolate.

If at the same time thread B uses the Locker and enters the isolate, we end up with two threads using the isolate simultaneously.

[Meir Shpilraien]

I actually do not have such a scenario, for me its enough to exit and enter once.

But I do think that the API is confusing, I would expect the unlocker to handle it and exit completely from the isolate and re-enter to the same state after disposed.

I also wonder why I need to explicitly exit/enter the isolate? Is there a case where I would want to create an unlocker without exit the isolate?

[Nikos Papaspyrou]

IMO, there is indeed an inconsistency in the API and I would also expect locker/unlocker to automatically trigger isolate enter/exit.

However, it is not a priority for us now to change this behaviour.

The API stands like this for a long time, chromium uses v8::Unlocker only minimally and we are unsure of how it is used by external embedders and what would break for them if we changed it.

All this said, if there is a concrete suggestion, of course we accept patches.