Fix Syncvar and FEB Lock/Unlock In Different Threads #207
Conversation
|
Found the corresponding lock to this unlock. The issue wasn't that the unlock was unpaired it was that the unlock was getting called from a completely different thread. The fix for that will be to either move the lock/unlock pairing either entirely into the called thread or entirely outside of it. I'm favoring locking outside of it here, but I'm not entirely sure of the performance implications of lock placement here so it'd be great to get a second opinion on that. |
|
Okay, moving the lock/unlock to the outer scope caused a deadlock with the nemesis/hwloc config, so apparently it has to go inside the called function. |
|
Hmm. Moving it inside may not work either under some circumstances even though that was working locally. Marking as WIP for now so I can decide what to do with it. |
insertinterestingnamehere
force-pushed
the
syncvar_lock
branch
from
2cb239f
to
134ac4d
Compare
insertinterestingnamehere
changed the base branch from
main
to
release-1.20-pre
insertinterestingnamehere
force-pushed
the
syncvar_lock
branch
2 times, most recently
from
ec127fe
to
ad5a01d
Compare
insertinterestingnamehere
force-pushed
the
syncvar_lock
branch
from
9608314
to
e401489
Compare
insertinterestingnamehere
changed the title
|
Alright! I sorted through what was going on here. This idiom was using a pthread mutex as a way to notify a waiting external thread that a blocked call had completed. Thread sanitizer was complaining since paired calls to lock/unlock from different threads aren't supposed to happen. Fortunately this is a fairly straightforward application of a condition variable so I just switched to using one of those. |
|
Okay, latest CI runs are showing some failures with the distrib scheduler now. Investigating... |
|
I can't reproduce this locally. I'll set it down for now and circle back to it later. |
|
What's the status of this? |
|
This one's good to go now. It was causing some lingering issues in CI last time you looked at it, but I've fixed those since then. |
insertinterestingnamehere
merged commit 4b54807
into
sandialabs:release-1.20-pre
Fixes #202.
As best I can tell, this function is only ever called in one place and the lock is managed by the caller there, so no unlock is needed. It's probably a leftover typo from some previous refactoring.