ldms_stream_close race fix #1294
Merged
Conversation
This file contains 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
ldms.Xprt.stream_unsubscribe() held GIL (Python's Global Interpreter Lock) while it `sem_wait()` for the result. This resulted in a dead lock on the GIL. This patch removed the unnecessary GIL holding in ldms.Xprt.stream_unsubscribe() function.
This is used in test script to control DEBUG logging (e.g. for verification).
Stream-Client-Entry (sce) is a tailq entry that lives in `stream->client_tq` (list of clients subscribing the stream) and `client->stream_tq` (list of streams the client subscribed to) to manage client-stream relation. When a stream client closes, it is unbound from the stream, and the `sce` is removed from the list. This leads to a use-after-free when another thread iterate through the `stream->client_tq` and unlock to call the application callback function. This patch modifies the unbind procedure to NOT remove the `sce`, and just unbind the client - stream from the `sce`. The unbound `sce` is later removed from the `client->stream_tq` list when the client is closed, and is removed from `stream->client_tq` when the callback processing thread detects an unbound `sce`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 pull request includes
ldms_stream_close()race fixldms.Xprt.stream_unscribe()GIL deadlock fixovis_log_set_level_by_name()to Python ldms for debugging / verification purpose.The following is the description of the
ldms_stream_close()race:Stream-Client-Entry (sce) is a tailq entry that lives in
stream->client_tq(list of clients subscribing the stream) andclient->stream_tq(list of streams the client subscribed to) tomanage client-stream relation. When a stream client closes, it is
unbound from the stream, and the
sceis removed from the list. Thisleads to a use-after-free when another thread iterate through the
stream->client_tqand unlock to call the application callbackfunction. This patch modifies the unbind procedure to NOT remove the
sce, and just unbind the client - stream from thesce. The unboundsceis later removed from theclient->stream_tqlist when the clientis closed, and is removed from
stream->client_tqwhen the callbackprocessing thread detects an unbound
sce.