remove terminate from demo #5959
Merged
+0
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was causing deadlocks because: 1. The main thread would `thread.terminate` a running thread 2. `thread.terminate` does not mean termination happens immediately 3. The thread that was terminated would see that the main thread released `print_mutex` and acquire it 4. The worker would execute `fmt.printf` which is a cancellation point 5. Cancellation point reached, cancelled 6. Deadlock because the thread was holding `print_mutex` You would usually solve this with `pthread_cleanup_push` that would release the mutex in case it is cancelled, or use `pthread_setcancelstate` to disable cancellation while the mutex is held. But the real fix is just not using forced termination and using a flag or other mechanism to indicate to the thread it should stop. `thread.terminate` shouldn't even be a thing IMO. It is way to dangerous and if somebody knows what they are doing they can use the core:sys procedures to achieve it. And we certainly shouldn't be using it in the demo as an example.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was causing deadlocks because:
thread.terminatea running threadthread.terminatedoes not mean termination happens immediatelyprint_mutexand acquire itfmt.printfwhich is a cancellation pointprint_mutexYou would usually solve this with
pthread_cleanup_pushthat would release the mutex in case it is cancelled, or usepthread_setcancelstateto disable cancellation while the mutex is held. But the real fix is just not using forced termination and using a flag or other mechanism to indicate to the thread it should stop.thread.terminateshouldn't even be a thing IMO.It is way to dangerous and if somebody knows what they are doing they can use the core:sys procedures to achieve it.
And we certainly shouldn't be using it in the demo as an example.