Skip to content

Conversation

@shinrich
Copy link
Member

@shinrich shinrich commented Feb 5, 2020

If a continuation does not have a mutex and it is being scheduled, the original logic will assign the mutex associated with current net handler thread. If this is a long-lived, global continuation, assigning a thread's net handler mutex will cause lock contention blocking other threads. It will also mean that it is probable that the nethandler lock will not be immediately acquired as in the scenario described in PR #5950.

This PR changes the logic to allow the continuation mutex to be null during scheduling, with the assumption that the plugin writer is dealing with locking directly in the plugin. It does issue a warning so the operations team can review logs for surprising unlocked, scheduled plugins. It also changes the lock acquisition in process_event to a weak lock to allow for the possibility of a null mutex.

We have been running with this change since after the Christmas break and it eliminated crashes due to the assert I added that was detecting high contention continuation locks (presumably due to use of nethandler mutexes on the continuation).

@shinrich shinrich added this to the 10.0.0 milestone Feb 5, 2020
@shinrich shinrich self-assigned this Feb 5, 2020
@shinrich
Copy link
Member Author

shinrich commented Feb 5, 2020

This solution should address issue #5944

@shinrich shinrich force-pushed the avoid-mutex-conflicts branch from f402c7b to 8a572ee Compare February 5, 2020 16:18
@shinrich
Copy link
Member Author

shinrich commented Feb 5, 2020

[approve ci clang-analyzer]

@shinrich shinrich merged commit 9b78e8e into apache:master Feb 10, 2020
@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 Feb 11, 2020
@zwoop
Copy link
Contributor

zwoop commented Feb 11, 2020

Cherry-picked to v9.0.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants