[+Yang, Jakob]
On Mon, Dec 18, 2017 at 12:06 PM Ben Newman <[email protected]> wrote: > Hi folks! This is my first time posting here, so please let me know if I > should ask this question somewhere else instead. I'm asking here because > https://github.com/v8/v8 does not have GitHub issues enabled. > > In short, I have a project that embeds V8 and uses a single `Isolate` from > multiple threads. > > The program runs just fine, but sometimes the inspector doesn't stop on > the correct line after stepping over a statement that switches threads > behind the scenes, even though the original thread is restored by the time > the next statement is executed. > > From what I can tell, the key information that controls this behavior is > `thread_local_.target_frame_count_`, first set here > <https://github.com/v8/v8/blob/3928133c96c7e7e785b66b59d2a13f24d624ed75/src/debug/debug.cc#L968>, > and then checked here > <https://github.com/v8/v8/blob/3928133c96c7e7e785b66b59d2a13f24d624ed75/src/debug/debug.cc#L412> > . > > That check fails because `target_frame_count === -1`, which suggests it > has not been updated since it was last initialized here > <https://github.com/v8/v8/blob/3928133c96c7e7e785b66b59d2a13f24d624ed75/src/debug/debug.cc#L251> > . > > Digging a bit deeper, I realized that the `Debug::ArchiveDebug` and > `Debug::RestoreDebug` methods, which should be responsible for > saving/restoring this `ThreadLocal` information when switching threads, > currently don't do anything > <https://github.com/v8/v8/blob/3928133c96c7e7e785b66b59d2a13f24d624ed75/src/debug/debug.cc#L263-L274> > . > > I can understand that throwing away debugger state when switching threads > might be OK if no one needs to debug a multi-threaded V8 program, but I > think I've found a legitimate use case for preserving that state, so I > would like to submit a PR to fix this. > > The essence of the PR would be: > > char* Debug::ArchiveDebug(char* storage) { > MemCopy(storage, reinterpret_cast<char*>(&thread_local_), > ArchiveSpacePerThread()); > return storage + ArchiveSpacePerThread(); > } > > > char* Debug::RestoreDebug(char* storage) { > MemCopy(reinterpret_cast<char*>(&thread_local_), storage, > ArchiveSpacePerThread()); > return storage + ArchiveSpacePerThread(); > } > > > int Debug::ArchiveSpacePerThread() { > return sizeof(ThreadLocal); > } > > Would this be a welcome PR? Is there anyone in particular I should ask to > review this? Any other advice for a first-time contributor? > > Thanks, > Ben > > > -- > -- > v8-users mailing list > [email protected] > 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 [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- -- v8-users mailing list [email protected] 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
