[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

[Alex Langford via Phabricator via lldb-commits]

bulbazord requested changes to this revision.
bulbazord added a comment.
This revision now requires changes to proceed.

I think that the idea you've laid out in the summary makes sense but the implementation seems off.
My understanding of `std::condition_variable` is that combined with `std::mutex` it is used to facilitate inter-thread synchronization. That would make sense if `thread_1` and `thread_2` both used a mutex and condition_variable to communicate with each other about the status of `n` but it looks like only `thread_1` actually uses and manages the mutex/condition_variable. Why is `call_and_wait` invoking `notify_one` when it's also what invokes `baz` (which is what is invoking `wait`)? This seems somewhat sketchy because the thread that is performing the notifications is the same thread that is performing the wait.

To accomplish your goal, I think it would probably look more like this:

1. `compute_pow` should acquire the lock on the mutex, modify `n`, unlock, and then invoke `cv.notify_one()`.
2. `call_and_wait` should only invoke `baz`. I don't think that the implementation of `baz` would require any modifications. Maybe `thread_2` could run `baz` directly instead?

Perhaps I'm mistaken about some details but I think this could probably be written in a clearer way.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139484/new/

https://reviews.llvm.org/D139484