-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ThreadSanitizer: data race at PlatformWorkerThread::trace_event_unique_atomic35 #26100
Comments
valgrind has been complaining about these too – I couldn’t find anything while looking into it, and it seemed like valgrind just didn’t get that atomics were being used here and that that was okay. But maybe somebody from @nodejs/trace-events (@ofrobots ?) could verify that? |
Note that there are several issues TSan is complaining about. Two of which are in libuv. |
I looked into it a bit more and it turns out that we’re not actually using atomics, we just gave everything a name with node/src/tracing/trace_event.h Lines 130 to 133 in e74019f
|
And the libuv issues seem to also be something I’ve seen before (with helgrind), namely that thread sanity checkers don’t see the causal relation between |
Use actual atomic operations instead of things that are named as if they were atomics, but aren’t. Refs: nodejs#26100
I'll retry after blacklisting the libuv issues :) |
Use actual atomic operations instead of things that are named as if they were atomics, but aren’t. Refs: #26100 PR-URL: #26156 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Yang Guo <yangguo@chromium.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Use actual atomic operations instead of things that are named as if they were atomics, but aren’t. Refs: #26100 PR-URL: #26156 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Yang Guo <yangguo@chromium.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Use actual atomic operations instead of things that are named as if they were atomics, but aren’t. Refs: #26100 PR-URL: #26156 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Yang Guo <yangguo@chromium.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
ping @addaleax |
I'm going to assume enough time has passed that these have been resolved. If not, please reopen. |
I attempted to build Node.js with TSAN, and it seems to have found something. This happens at the build step where we run node to create code cache for internal libs. At this point I'm not sure whether this is an actual issue, or whether Node.js' code is just too clever for TSAN to understand, and we need to put suppressions in place. Thought I'd share.
Steps to reproduce:
Unfortunately the TSAN build is only available to GN.
get depot_tools
The text was updated successfully, but these errors were encountered: