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

Ensure we use a MPMC queue in ThreadDeathWatcher as it may be used fr… #6149

Closed
wants to merge 1 commit into from

Conversation

@normanmaurer
Copy link
Member

commented Dec 20, 2016

…om multiple threads at the same time.

Motivation:

We used a MPSC queue in ThreadDeathWatcher and checked if it empty via isEmpty() from multiple threads if very unlucky. Depending on the implementation this is not safe and may even produce things like live-locks.

Modifications:

Change to use a MPMC queue.

Result:

No more risk to run into issues when multiple threads call watch(...) / unwatch(...) concurrently.

Ensure we use a MPMC queue in ThreadDeathWatcher as it may be used fr…
…om multiple threads at the same time.

Motivation:

We used a MPSC queue in ThreadDeathWatcher and checked if it empty via isEmpty() from multiple threads if very unlucky. Depending on the implementation this is not safe and may even produce things like live-locks.

Modifications:

Change to use a MPMC queue.

Result:

No more risk to run into issues when multiple threads call watch(...) / unwatch(...) concurrently.
@Scottmitch
Copy link
Member

left a comment

This change LGTM ... however the larger algorithm of this class still gives me heartburn.

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2016

@Scottmitch agree.. Followup PR FTW ;)

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2016

Cherry-picked into 4.1 (28c39a3) and 4.0 (c7dbb7b)

@normanmaurer normanmaurer deleted the thread_death_queue branch Dec 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.