Bug 282396 – Usage of AtomString in TrackPrivateBaseGStreamer is not thread-safe

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

Currently TrackPrivateBaseGStreamer uses AtomString in these fields:

    AtomString m_label;
    AtomString m_language;
    AtomString m_stringId;

Additionally, AtomString is received and returned by several methods.

    const AtomString& stringId() const { return m_stringId; };
private:
    static AtomString generateUniquePlaybin2StreamID(TrackType, unsigned index);
    static TrackID trackIdFromStringIdOrIndex(TrackType, const AtomString&, unsigned);

Unfortunately, AtomString is not thread-safe in _at least_ two use cases.

Use case #1: AtomStrings created in one thread cannot be destroyed in another.

I saw this happen in imported/w3c/web-platform-tests/media-source/mediasource-sequencemode-append-buffer.html within a Debug build. It crashed inside the destructor of one of the AtomStrings of TrackPrivateBaseGStreamer, in this assert:

void AtomStringImpl::remove(AtomStringImpl* string)
{
// ...
    ASSERT_WITH_MESSAGE(iterator != atomStringTable.end(), "The string being removed is an atom in the string table of an other thread!");

Use case #2: The internal ref counting of AtomStrings is also not thread-safe, as found by Eugene: https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1372/commits/33bfbb48666b7de47041e021654894d840b51ff4

[Alicia Boya García]

Pull request: https://github.com/WebKit/WebKit/pull/36378

[EWS]

Committed 286324@main (d6348a3fcc70): <https://commits.webkit.org/286324@main>

Reviewed commits have been landed. Closing PR #36378 and removing active labels.