Fix MoodbarPipeline crash on gstreamer error. #6308
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
As reported in issue 6302, playing a stream that causes gstreamer to error at start can cause a crash. The problem occurs when the MoodbarPipeline receives a pad-added signal after it has handled an error callback. In the error callback, the builder_ is freed. In the pad-added handler (NewPadCallback), this object is accessed. This change adds a running_ flag that is set when the pipeline is started and cleared on an error, end of stream, or object destruction. We check this flag at the beginning of NewPadCallback. For sanity sake, we also check the builder_ pointer before dereferencing. Note that checking the state of the pipeline wasn't an option since the pipeline is in the process of changing states during the pad-added callback and gst_element_get_state wants to block during a state change. This solution is not complete as there are still some syncronization issues. With this specific situation, the error and new pad callbacks appear to always occur on the same thread, but that's probably not true for all error conditions. The object is also destroyed by a different thread, so it may be possible that a callback can occur at the wrong time during or after the deletion of the object. See clementine-player#6302
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.
As reported in issue 6302, playing a stream that causes gstreamer to error at
start can cause a crash. The problem occurs when the MoodbarPipeline receives a
pad-added signal after it has handled an error callback. In the error callback,
the builder_ is freed. In the pad-added handler (NewPadCallback), this object
is accessed.
This change adds a running_ flag that is set when the pipeline is started and
cleared on an error, end of stream, or object destruction. We check this flag at
the beginning of NewPadCallback. For sanity sake, we also check the builder_
pointer before dereferencing. Note that checking the state of the pipeline
wasn't an option since the pipeline is in the process of changing states during
the pad-added callback and gst_element_get_state wants to block during a state
change.
This solution is not complete as there are still some syncronization issues.
With this specific situation, the error and new pad callbacks appear to always
occur on the same thread, but that's probably not true for all error conditions.
The object is also destroyed by a different thread, so it may be possible that a
callback can occur at the wrong time during or after the deletion of the object.
See #6302