⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 264648 in webkit


Ignore:
Timestamp:
Jul 21, 2020, 2:03:00 AM (6 years ago)
Author:
calvaris@igalia.com
Message:

[GStreamer][1.18] mediastreamsrc element hits assert in gst -core
​https://bugs.webkit.org/show_bug.cgi?id=214150

Reviewed by Philippe Normand.

Source/WebCore:

Removed track pointers from observers cause the object was
self-referencing a couple of times. Observers are 1-1 with the
source so there is no need to keep an owned reference and there is
no danger of incorrect dereferencing.

Turned finalize into dispose as the work is doing is more removing
the references with other objects, which what the dispose is
about. Called the parent on that, which as missing as well.

  • platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:

(WebKitMediaStreamTrackObserver::WebKitMediaStreamTrackObserver):
(WebKitMediaStreamObserver::WebKitMediaStreamObserver):
(webkitMediaStreamSrcConstructed):
(webkitMediaStreamSrcDispose):
(webkit_media_stream_src_class_init):

LayoutTests:

As result of this bug
fast/mediastream/MediaStream-removeTrack-while-playing.html begins
to crash in debug mode in an assertion. When a track is removed
the observer is not removed and when it dies there are null
objects in the observer weak hash set. Still, if you do a quick
patch to remove the observer, things can still crash as track
removal is racy and in release there' other crash. All this needs
to be taclked as a general approach to fix track removal.

  • platform/glib/TestExpectations: Added

fast/mediastream/MediaStream-removeTrack-while-playing.html as
Pass Crash for release and crash for debug.

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r264646 r264648  
     12020-07-21  Xabier Rodriguez Calvar  <calvaris@igalia.com>
     2
     3        [GStreamer][1.18] mediastreamsrc element hits assert in gst -core
     4        https://bugs.webkit.org/show_bug.cgi?id=214150
     5
     6        Reviewed by Philippe Normand.
     7
     8        As result of this bug
     9        fast/mediastream/MediaStream-removeTrack-while-playing.html begins
     10        to crash in debug mode in an assertion. When a track is removed
     11        the observer is not removed and when it dies there are null
     12        objects in the observer weak hash set. Still, if you do a quick
     13        patch to remove the observer, things can still crash as track
     14        removal is racy and in release there' other crash. All this needs
     15        to be taclked as a general approach to fix track removal.
     16
     17        * platform/glib/TestExpectations: Added
     18        fast/mediastream/MediaStream-removeTrack-while-playing.html as
     19        Pass Crash for release and crash for debug.
     20
    1212020-07-21  Carlos Garcia Campos  <cgarcia@igalia.com>
    222

  • trunk/LayoutTests/platform/glib/TestExpectations

    r264595 r264648  
    194194webkit.org/b/187603 fast/mediastream/media-stream-track-source-failure.html [ Timeout Failure Pass ]
    195195webkit.org/b/191886 webkit.org/b/210800 fast/mediastream/MediaStream-video-element-remove-track.html [ Failure Crash ]
     196webkit.org/b/191886 webkit.org/b/210800 [ Release ] fast/mediastream/MediaStream-removeTrack-while-playing.html [ Pass Crash ]
     197webkit.org/b/191886 webkit.org/b/210800 [ Debug ] fast/mediastream/MediaStream-removeTrack-while-playing.html [ Crash ]
    196198webkit.org/b/199440 fast/mediastream/mediastreamtrack-video-clone.html [ Crash Pass ]
    197199webkit.org/b/210337 fast/mediastream/mediastreamtrack-audio-clone.html [ Failure Pass ]

  • trunk/Source/WebCore/ChangeLog

    r264646 r264648  
     12020-07-21  Xabier Rodriguez Calvar  <calvaris@igalia.com>
     2
     3        [GStreamer][1.18] mediastreamsrc element hits assert in gst -core
     4        https://bugs.webkit.org/show_bug.cgi?id=214150
     5
     6        Reviewed by Philippe Normand.
     7
     8        Removed track pointers from observers cause the object was
     9        self-referencing a couple of times. Observers are 1-1 with the
     10        source so there is no need to keep an owned reference and there is
     11        no danger of incorrect dereferencing.
     12
     13        Turned finalize into dispose as the work is doing is more removing
     14        the references with other objects, which what the dispose is
     15        about. Called the parent on that, which as missing as well.
     16
     17        * platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
     18        (WebKitMediaStreamTrackObserver::WebKitMediaStreamTrackObserver):
     19        (WebKitMediaStreamObserver::WebKitMediaStreamObserver):
     20        (webkitMediaStreamSrcConstructed):
     21        (webkitMediaStreamSrcDispose):
     22        (webkit_media_stream_src_class_init):
     23
    1242020-07-21  Carlos Garcia Campos  <cgarcia@igalia.com>
    225

  • trunk/Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp

    r264607 r264648  
    101101public:
    102102    virtual ~WebKitMediaStreamTrackObserver() { };
    103     WebKitMediaStreamTrackObserver(GRefPtr<GstElement>&& src)
    104         : m_src(WTFMove(src)) { }
     103    WebKitMediaStreamTrackObserver(GstElement* src)
     104        : m_src(src) { }
    105105    void trackStarted(MediaStreamTrackPrivate&) final { };
    106106
    … …  
    108108    {
    109109        if (m_src)
    110             webkitMediaStreamSrcTrackEnded(WEBKIT_MEDIA_STREAM_SRC(m_src.get()), track);
     110            webkitMediaStreamSrcTrackEnded(WEBKIT_MEDIA_STREAM_SRC(m_src), track);
    111111    }
    112112
    … …  
    126126
    127127        auto* gstSample = static_cast<MediaSampleGStreamer*>(&sample)->platformSample().sample.gstSample;
    128         webkitMediaStreamSrcPushVideoSample(WEBKIT_MEDIA_STREAM_SRC(m_src.get()), gstSample);
     128        webkitMediaStreamSrcPushVideoSample(WEBKIT_MEDIA_STREAM_SRC(m_src), gstSample);
    129129    }
    130130
    … …  
    135135
    136136        auto data = static_cast<const GStreamerAudioData&>(audioData);
    137         webkitMediaStreamSrcPushAudioSample(WEBKIT_MEDIA_STREAM_SRC(m_src.get()), data.getSample());
     137        webkitMediaStreamSrcPushAudioSample(WEBKIT_MEDIA_STREAM_SRC(m_src), data.getSample());
    138138    }
    139139
    140140private:
    141     GRefPtr<GstElement> m_src;
     141    GstElement* m_src;
    142142    bool m_enabled { true };
    143143};
    … …  
    147147public:
    148148    virtual ~WebKitMediaStreamObserver() { };
    149     WebKitMediaStreamObserver(GRefPtr<GstElement>&& src)
    150         : m_src(WTFMove(src)) { }
     149    WebKitMediaStreamObserver(GstElement* src)
     150        : m_src(src) { }
    151151
    152152    void characteristicsChanged() final
    153153    {
    154154        if (m_src)
    155             GST_DEBUG_OBJECT(m_src.get(), "renegotiation should happen");
     155            GST_DEBUG_OBJECT(m_src, "renegotiation should happen");
    156156    }
    157157    void activeStatusChanged() final { }
    … …  
    160160    {
    161161        if (m_src)
    162             webkitMediaStreamSrcAddTrack(WEBKIT_MEDIA_STREAM_SRC(m_src.get()), &track, false);
     162            webkitMediaStreamSrcAddTrack(WEBKIT_MEDIA_STREAM_SRC(m_src), &track, false);
    163163    }
    164164
    … …  
    166166    {
    167167        if (m_src)
    168             webkitMediaStreamSrcRemoveTrackByType(WEBKIT_MEDIA_STREAM_SRC(m_src.get()), track.type());
     168            webkitMediaStreamSrcRemoveTrackByType(WEBKIT_MEDIA_STREAM_SRC(m_src), track.type());
    169169    }
    170170
    171171private:
    172     GRefPtr<GstElement> m_src;
     172    GstElement* m_src;
    173173};
    174174
    … …  
    341341    auto* priv = self->priv;
    342342
    343     priv->mediaStreamTrackObserver = makeUnique<WebKitMediaStreamTrackObserver>(GST_ELEMENT_CAST(gst_object_ref(self)));
    344     priv->mediaStreamObserver = makeUnique<WebKitMediaStreamObserver>(GST_ELEMENT_CAST(gst_object_ref(self)));
     343    priv->mediaStreamTrackObserver = makeUnique<WebKitMediaStreamTrackObserver>(GST_ELEMENT_CAST(self));
     344    priv->mediaStreamObserver = makeUnique<WebKitMediaStreamObserver>(GST_ELEMENT_CAST(self));
    345345    priv->flowCombiner = GUniquePtr<GstFlowCombiner>(gst_flow_combiner_new());
     346
     347    // https://bugs.webkit.org/show_bug.cgi?id=214150
     348    ASSERT(GST_OBJECT_REFCOUNT(self) == 1);
     349    ASSERT(g_object_is_floating(self));
    346350}
    347351
    … …  
    364368}
    365369
    366 static void webkitMediaStreamSrcFinalize(GObject* object)
     370static void webkitMediaStreamSrcDispose(GObject* object)
    367371{
    368372    WebKitMediaStreamSrc* self = WEBKIT_MEDIA_STREAM_SRC(object);
    … …  
    376380        priv->stream = nullptr;
    377381    }
     382    priv->track = nullptr;
    378383    GST_OBJECT_UNLOCK(self);
     384
     385    GST_CALL_PARENT(G_OBJECT_CLASS, dispose, (object));
    379386}
    380387
    … …  
    402409
    403410    gobjectClass->constructed = webkitMediaStreamSrcConstructed;
    404     gobjectClass->finalize = webkitMediaStreamSrcFinalize;
     411    gobjectClass->dispose = webkitMediaStreamSrcDispose;
    405412    gobjectClass->get_property = webkitMediaStreamSrcGetProperty;
    406413    gobjectClass->set_property = webkitMediaStreamSrcSetProperty;
Note: See TracChangeset for help on using the changeset viewer.