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

Description Michael Catanzaro 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.
Comment 1 Philippe Normand 2023-06-15 07:49:56 PDT 
Pull request: https://github.com/WebKit/WebKit/pull/15003
Comment 2 EWS 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.
Comment 3 Radar WebKit Bug Importer 2023-06-16 03:03:22 PDT 
<rdar://problem/110891059>