Skip to content
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

Large number of data races in unit tests #1615

Open
NAThompson opened this issue Jul 17, 2019 · 5 comments
Open

Large number of data races in unit tests #1615

NAThompson opened this issue Jul 17, 2019 · 5 comments
Assignees
Labels
bug triage: medium This issue is important to be fixed but can currently be worked around, even if poorly

Comments

@NAThompson
Copy link
Contributor

In preparation for parallel reads, I added the following to the CMakeLists.txt and ran the unit tests:

if(CMAKE_BUILD_TYPE MATCHES "Debug")
  if(CMAKE_CXX_COMPILER_ID MATCHES "^(AppleClang|Clang|GNU)")
    set(CMAKE_CXX_FLAGS_DEBUG "-Wall -g -O2 -Wfatal-errors -fsanitize=thread -fno-omit-frame-pointer")
  endif()
endif()

A very large number of race conditions were detected. A few were in pthread_create, which arguably is bad, but not the bailiwick of ADIOS. But a large number of them are originating in ADIOS2. I have attached just one of these here, but many tests are afflicted by these problems.

race.txt

@pnorbert
Copy link
Contributor

This has just started showing up in CI. The parsing of variables is done in multiple threads, and they all create a new variable and add to the same variable map using map.emplace().

./bin/TestBPWriteReadAsStreamADIOS2_Threads
both BP3 and BP4

@lwan86
Copy link
Contributor

lwan86 commented Jul 19, 2019

@pnorbert Could you let me know how to reproduce this bug? I run "TestBPWriteReadAsStreamADIOS2_Threads" several times, it works fine.

@williamfgc
Copy link
Contributor

@NAThompson thanks, we need to start paying more attention to fsanitize options. @pnorbert this is probably related to #1125 . I will work on a fix.

@chuckatkins
Copy link
Contributor

We currently have ubsan running in the CI for dynamic analysis and we can just as easily add a tsan build. Once we get a clean tsan build then I can add it to our ci checks.

@NAThompson
Copy link
Contributor Author

@chuckatkins , @williamfgc , @pnorbert : Huge thanks to all of you for taking this seriously. As William notes, you have to work on what's a priority, and being race-clean is mine-but you guys are subjected to the work! So it's huge for me to see PRs getting merged that address this.

@chuckatkins chuckatkins added the triage: medium This issue is important to be fixed but can currently be worked around, even if poorly label Jul 30, 2019
@williamfgc williamfgc mentioned this issue Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage: medium This issue is important to be fixed but can currently be worked around, even if poorly
Projects
None yet
Development

No branches or pull requests

5 participants