Bug 258129

Summary: REGRESSION(264515@main): [GStreamer] Use of WeakPtr for m_player in MediaPlayerPrivateGStreamer looks unsafe
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: MediaAssignee: Philippe Normand <philn>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=254954
https://bugs.webkit.org/show_bug.cgi?id=258022

Michael Catanzaro
Reported 2023-06-15 07:02:02 PDT
Moving this from bug #254954. I'm not sure that changing m_player to be a WeakPtr is actually safe to do without auditing its usage further. For example, in MediaPlayerPrivateGStreamer::handleNeedContextMessage, we have: gst_structure_set(contextStructure, "player", G_TYPE_POINTER, m_player.get(), nullptr); gst_element_set_context(GST_ELEMENT(GST_MESSAGE_SRC(message)), context.get()); So now it's stored dereferenced for what I assume is the lifetime of this GstMessage, and the WeakPtr provides no protection against invalidation. So probably it's not safe to use WeakPtr. This should be looked at more closely.
Attachments
Philippe Normand
Comment 1 2023-06-15 07:49:56 PDT
EWS
Comment 2 2023-06-16 03:02:14 PDT
Committed 265236@main (1ddda51c8059): <https://commits.webkit.org/265236@main> Reviewed commits have been landed. Closing PR #15003 and removing active labels.
Radar WebKit Bug Importer
Comment 3 2023-06-16 03:03:22 PDT
Note You need to log in before you can comment on or make changes to this bug.