V8's API is generally single-threaded and all cross-thread access to Isolates must go through using v8::Locker.
-Michael On Fri, Jun 7, 2024 at 11:42 AM Sam Cao <sjtucao...@gmail.com> wrote: > Hi Dominik, > > Thank you for the suggestion. I prefer a feature request. > > Calling that API in the same thread might not be possible. In the use case > I left, that thread is busy allocating memory so that there's no way of > executing that API. Why do we want to know the heap statistics? Because > that thread hangs and we want to know why. This is a typical use case in > Java, C#, etc. where we rely on JVM / .Net runtime statistics collected by > a troubleshooting thread. > > Thank you again and have a nice weekend, > Sam > > On Friday, June 7, 2024 at 5:24:48 PM UTC+8 dinf...@chromium.org wrote: > >> Hi Sam, >> >> You should definitely file a feature request for that. I understand your >> use case and I see how one could assume that this would work. I suppose the >> behavior change comes from this CL in particular [0]. And that change was >> made under the assumption that calls would be on the right thread. There >> may be other (less obvious) issues with this method on concurrent threads >> as well. So if we want to support concurrent usage we should check the rest >> of the method as well and have some test coverage for this. Alternatively >> you could also invoke that method on the right thread (e.g. through posting >> a task) and let it report back the result as well. >> >> Cheers, >> Dominik >> >> 0: https://chromium-review.googlesource.com/c/v8/v8/+/4946715 >> >> On Friday, June 7, 2024 at 10:24:22 AM UTC+2 sjtuc...@gmail.com wrote: >> >>> Hi Dominik, >>> >>> I forgot to mention the OS is Windows 10 x86_64. Monitoring the heap >>> statistics in another thread used to work well before this change. >>> >>> I wonder why this call has to be in the same thread. Consider V8 is used >>> in an online service (Node.js). The service provider usually owns a >>> dashboard showing the server farm statistics including the Node.js heap >>> usage. I'm afraid the data has to be coming from another thread hitting the >>> same V8 isolate via this call. >>> >>> For now, the Node.js v20.14.0 LTS (V8 v11.3.244.8) hasn't picked up this >>> change, so this issue doesn't impact the majority of users. I removed this >>> check in my private build and this API works as usual now. >>> >>> Please consider this feature as one of the key features in online >>> services and find a better way to resolve it. >>> >>> Thank you, >>> Sam >>> >>> >>> On Friday, June 7, 2024 at 3:54:20 PM UTC+8 dinf...@chromium.org wrote: >>> >>>> Hi, >>>> >>>> I cannot reproduce this crash locally (neither on 12.5.227.6 nor on >>>> tip-of-tree). I suspect what's going on here is that you are using >>>> GetHeapStatistics from a second thread which is not supported. This likely >>>> causes this particular CHECK failure. The fix for this would be to invoke >>>> GetHeapStatistics on thread 1 as well. >>>> >>>> Cheers, >>>> Dominik >>>> >>>> >>>> On Friday, June 7, 2024 at 6:22:01 AM UTC+2 sjtuc...@gmail.com wrote: >>>> >>>>> Hi V8 Dev, >>>>> >>>>> I'd like to report a potential bug in ConcurrentMarking::RunMajor(). >>>>> >>>>> *Fatal Error* >>>>> # >>>>> # Fatal error in , line 0 >>>>> # Check failed: !IsFreeSpaceOrFillerMap(map). >>>>> # >>>>> # >>>>> # >>>>> #FailureMessage Object: 00000083D21FF440 >>>>> ==== C stack trace =============================== >>>>> CrashForExceptionInNonABICompliantCodeRange [0x00007FFF4AA557BB+ >>>>> 1514667] >>>>> (No symbol) [0x00007FFF4A77D497] >>>>> (No symbol) [0x00007FFF4A820BBA] >>>>> CrashForExceptionInNonABICompliantCodeRange [0x00007FFF4AB234A1+ >>>>> 2357649] >>>>> CrashForExceptionInNonABICompliantCodeRange [0x00007FFF4AB3AB62+ >>>>> 2453586] >>>>> CrashForExceptionInNonABICompliantCodeRange [0x00007FFF4AA570A6+ >>>>> 1521046] >>>>> CrashForExceptionInNonABICompliantCodeRange [0x00007FFF4AA5A466+ >>>>> 1534294] >>>>> >>>>> *Reproduce* >>>>> >>>>> 1. Set max heap size to 8096 >>>>> 2. Start thread 1 and execute the following JS code. >>>>> var a = []; >>>>> for (let i = 0; i < 100000000; i++) { >>>>> a.push({test:'test'}); >>>>> } >>>>> 3. Start thread 2 and call v8Isolate->GetHeapStatistics() >>>>> periodically. >>>>> >>>>> There is a high chance that V8 will crash with the fatal error posted >>>>> above. >>>>> >>>>> *Analysis* >>>>> I reviewed the source code of 12.5.227.6 and found there is only one >>>>> call to IsFreeSpaceOrFillerMap() inside ConcurrentMarking::RunMajor() as >>>>> follows. >>>>> [image: 01.png] >>>>> >>>>> It seems this check is not always valid when that V8 isolate is busy >>>>> allocating memory. It used to be working well before this check was added. >>>>> >>>>> Please check this issue out. >>>>> >>>>> Thank you, >>>>> Sam >>>>> >>>> -- > -- > v8-dev mailing list > v8-dev@googlegroups.com > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to v8-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/v8-dev/09463827-7e16-459a-a12e-c470e5888e83n%40googlegroups.com > <https://groups.google.com/d/msgid/v8-dev/09463827-7e16-459a-a12e-c470e5888e83n%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAH%2BmL5CXxwtTSddZfOUhafY7DYttMLAO9qdOmgn851tqr8EXPg%40mail.gmail.com.
