SCP: Acqire mutexes for both cond signals and waits #516
Conversation
Acquire enclosing mutex used with pthread_cond_wait() when calling pthread_cond_signal() or pthread_cond_broadcast(). This eliminates the possibility that pthread_cond_wait() misses the corresponding pthread_- cond_signal(). See https://stackoverflow.com/questions/4544234/ for a good diagram that explains the thread interleaving. Also look for premature mutex unlocks. As the Linux pthread_cond_- signal() man page states, "... if predictable scheduling behavior is required, then that mutex shall be locked by the thread calling pthread_cond_broadcast() or pthread_cond_signal()." ETH: - Add startup condvar and mutex, have _eth_reader and _eth_writer signal successful thread startup (as other SIMH threads do.) - Set thread names for reader and writer threads.
|
Here we are again, with another PR that changes code which absolutely isn't demonstrably broken. If this was indeed broken, real failures would have arisen in the more than 20 years it has existed this way. Each time, you come along and propose changes to things that are not broken during any actual case of simulator operation, I would absolutely support such changes if you could actually demonstrate an operational failure in any running simulator situation, or if the events that actually happen are so rare in normal operation, but the theory still might be useful, you can readily implement a unit test using the existing simh TESTLIB framework that creates the conditions that demonstrate the failure and that with proposed changes avoids such a failure. You've previously added one such TESTLIB test, so the concept of unit testing isn't beyond you. Otherwise, you're making widespread changes merely for the sake of change since if you were writing things from scratch, that is how you would do it. Please show actual failure details that are then fixed and we can all get excited. Meanwhile, your commit (and PR) comments mention changes to sim_ether (ETH), but the actual changes you're providing sneak in changes to 8 additional (and unrelated) source modules. |
|
@markpizz Wouldn't have submitted a patch to correctly use Maybe fix your shit before shitting on others. |
|
Before you "fix" my shit, once again, it would be useful for everyone if you actually demonstrate that an actual bug exists AND the fact that such a bug is fixed by your efforts. |
2 participants
Acquire enclosing mutex used with pthread_cond_wait() when calling pthread_cond_signal() or pthread_cond_broadcast(). This eliminates the possibility that pthread_cond_wait() misses the corresponding pthread_- cond_signal(). See https://stackoverflow.com/questions/4544234/ for a good diagram that explains the thread interleaving.
Also look for premature mutex unlocks. As the Linux pthread_cond_- signal() man page states, "... if predictable scheduling behavior is required, then that mutex shall be locked by the thread calling pthread_cond_broadcast() or pthread_cond_signal()."
ETH:
Add startup condvar and mutex, have _eth_reader and _eth_writer signal successful thread startup (as other SIMH threads do.)
Set thread names for reader and writer threads.