Bug 282396
| Summary: | Usage of AtomString in TrackPrivateBaseGStreamer is not thread-safe | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Alicia Boya García <aboya> |
| Component: | WebKitGTK | Assignee: | Alicia Boya García <aboya> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | bugs-noreply |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
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
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
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.