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

Thread Sanitizer Interop #249

Conversation

insertinterestingnamehere
Copy link
Collaborator

This adds the API calls needed to help the thread sanitizer correctly carry its happens-before relationships across context swaps.

@insertinterestingnamehere
Copy link
Collaborator Author

Okay, doing another round of careful review of the fences in FEBs seems to have fixed the bugs I was trying to address here. On the other hand, this patch is still causing some mysterious issues. The interface I'm using is poorly documented so it's hard to say for sure if I'm using it correctly. I'm going to leave this open for a bit in case I need to come back to it, but for now I'm going to try to just move on without it.

@insertinterestingnamehere
Copy link
Collaborator Author

Rebased to depend on #256 to avoid conflicts.

@insertinterestingnamehere
Copy link
Collaborator Author

Okay, I think this is at least nominally working. There's something wrong where if we free the tsan fiber objects at the end it segfaults which is bizarre. Maybe something's getting double-freed during runtime cleanup.

@insertinterestingnamehere insertinterestingnamehere marked this pull request as ready for review April 8, 2024 17:14
@insertinterestingnamehere insertinterestingnamehere marked this pull request as draft April 8, 2024 17:21
@insertinterestingnamehere
Copy link
Collaborator Author

Nevermind, that was just a testing failure causing a false success. Marking this as a draft again for more debugging.

@insertinterestingnamehere insertinterestingnamehere marked this pull request as ready for review August 7, 2024 20:47
@insertinterestingnamehere
Copy link
Collaborator Author

Okay, this still needs some restructuring for clarity, but it is actually working now.

… context swaps.

Note: make stored tsan fibers atomic to clear a tsan error. This makes some sense because the fiber entry has to be accessed in order to prepare or finalize the context swap and those operations may happen in different OS threads when qthreads get stolen or just suspended for the IO subsystem.
@insertinterestingnamehere insertinterestingnamehere merged commit f499495 into sandialabs:release-1.21-pre Sep 27, 2024
239 of 342 checks passed
@insertinterestingnamehere
Copy link
Collaborator Author

Good enough. There's more to do to get thread sanitizer working correctly with our threadqueues, but this is a solid step forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant