Title: [277885] trunk/Source/WebKit
- Revision
- 277885
- Author
- [email protected]
- Date
- 2021-05-21 14:04:43 -0700 (Fri, 21 May 2021)
Log Message
Adopt CheckedLock in MediaFormatReader and fix threading bug
https://bugs.webkit.org/show_bug.cgi?id=226100
Reviewed by Darin Adler.
Adopt CheckedLock in MediaFormatReader and fix threading bug found by Clang Thread Safety
Analysis. In particular, parseByteSource() was failing to grab the lock before updating
m_parseTracksStatus in one of its branches.
* Shared/mac/MediaFormatReader/MediaFormatReader.cpp:
(WebKit::MediaFormatReader::parseByteSource):
(WebKit::MediaFormatReader::didParseTracks):
(WebKit::MediaFormatReader::didProvideMediaData):
(WebKit::MediaFormatReader::finishParsing):
(WebKit::MediaFormatReader::copyProperty):
(WebKit::MediaFormatReader::copyTrackArray):
* Shared/mac/MediaFormatReader/MediaFormatReader.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (277884 => 277885)
--- trunk/Source/WebKit/ChangeLog 2021-05-21 20:58:01 UTC (rev 277884)
+++ trunk/Source/WebKit/ChangeLog 2021-05-21 21:04:43 UTC (rev 277885)
@@ -1,5 +1,25 @@
2021-05-21 Chris Dumez <[email protected]>
+ Adopt CheckedLock in MediaFormatReader and fix threading bug
+ https://bugs.webkit.org/show_bug.cgi?id=226100
+
+ Reviewed by Darin Adler.
+
+ Adopt CheckedLock in MediaFormatReader and fix threading bug found by Clang Thread Safety
+ Analysis. In particular, parseByteSource() was failing to grab the lock before updating
+ m_parseTracksStatus in one of its branches.
+
+ * Shared/mac/MediaFormatReader/MediaFormatReader.cpp:
+ (WebKit::MediaFormatReader::parseByteSource):
+ (WebKit::MediaFormatReader::didParseTracks):
+ (WebKit::MediaFormatReader::didProvideMediaData):
+ (WebKit::MediaFormatReader::finishParsing):
+ (WebKit::MediaFormatReader::copyProperty):
+ (WebKit::MediaFormatReader::copyTrackArray):
+ * Shared/mac/MediaFormatReader/MediaFormatReader.h:
+
+2021-05-21 Chris Dumez <[email protected]>
+
[Cocoa] Unable to upload files that are stored in the cloud (without a local copy)
https://bugs.webkit.org/show_bug.cgi?id=226090
<rdar://77775887>
Modified: trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.cpp (277884 => 277885)
--- trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.cpp 2021-05-21 20:58:01 UTC (rev 277884)
+++ trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.cpp 2021-05-21 21:04:43 UTC (rev 277885)
@@ -100,6 +100,7 @@
static NeverDestroyed<ContentType> contentType("video/webm"_s);
auto parser = SourceBufferParserWebM::create(contentType);
if (!parser) {
+ Locker locker { m_parseTracksLock };
m_parseTracksStatus = kMTPluginFormatReaderError_AllocationFailure;
return;
}
@@ -131,7 +132,7 @@
didProvideMediaData(WTFMove(mediaSample), trackID, mediaType);
});
- auto locker = holdLock(m_parseTracksLock);
+ Locker locker { m_parseTracksLock };
m_byteSource = WTFMove(byteSource);
m_parseTracksStatus = WTF::nullopt;
m_duration = MediaTime::invalidTime();
@@ -149,7 +150,7 @@
{
ASSERT(!isMainRunLoop());
- auto locker = holdLock(m_parseTracksLock);
+ Locker locker { m_parseTracksLock };
ASSERT(!m_parseTracksStatus);
ASSERT(m_duration.isInvalid());
ASSERT(m_trackReaders.isEmpty());
@@ -192,7 +193,7 @@
{
ASSERT(!isMainRunLoop());
- auto locker = holdLock(m_parseTracksLock);
+ Locker locker { m_parseTracksLock };
auto trackIndex = m_trackReaders.findMatching([&](auto& track) {
return track->trackID() == trackID;
});
@@ -206,7 +207,7 @@
ASSERT(!isMainRunLoop());
ALWAYS_LOG(LOGIDENTIFIER);
- auto locker = holdLock(m_parseTracksLock);
+ Locker locker { m_parseTracksLock };
ASSERT(m_parseTracksStatus.hasValue());
for (auto& trackReader : m_trackReaders)
@@ -230,8 +231,9 @@
OSStatus MediaFormatReader::copyProperty(CFStringRef key, CFAllocatorRef allocator, void* valueCopy)
{
- auto locker = holdLock(m_parseTracksLock);
+ Locker locker { m_parseTracksLock };
m_parseTracksCondition.wait(m_parseTracksLock, [&] {
+ assertIsHeld(m_parseTracksLock);
return m_parseTracksStatus.hasValue();
});
@@ -251,8 +253,9 @@
OSStatus MediaFormatReader::copyTrackArray(CFArrayRef* trackArrayCopy)
{
- auto locker = holdLock(m_parseTracksLock);
+ Locker locker { m_parseTracksLock };
m_parseTracksCondition.wait(m_parseTracksLock, [&] {
+ assertIsHeld(m_parseTracksLock);
return m_parseTracksStatus.hasValue();
});
Modified: trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.h (277884 => 277885)
--- trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.h 2021-05-21 20:58:01 UTC (rev 277884)
+++ trunk/Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.h 2021-05-21 21:04:43 UTC (rev 277885)
@@ -29,8 +29,8 @@
#include "CoreMediaWrapped.h"
#include <WebCore/SourceBufferPrivateClient.h>
-#include <wtf/Condition.h>
-#include <wtf/Lock.h>
+#include <wtf/CheckedCondition.h>
+#include <wtf/CheckedLock.h>
#include <wtf/Logger.h>
DECLARE_CORE_MEDIA_TRAITS(FormatReader);
@@ -78,12 +78,12 @@
const void* logIdentifier() const { return m_logIdentifier; }
- RetainPtr<MTPluginByteSourceRef> m_byteSource;
- Condition m_parseTracksCondition;
- Lock m_parseTracksLock;
- MediaTime m_duration;
- Optional<OSStatus> m_parseTracksStatus;
- Vector<Ref<MediaTrackReader>> m_trackReaders;
+ RetainPtr<MTPluginByteSourceRef> m_byteSource WTF_GUARDED_BY_LOCK(m_parseTracksLock);
+ CheckedCondition m_parseTracksCondition;
+ CheckedLock m_parseTracksLock;
+ MediaTime m_duration WTF_GUARDED_BY_LOCK(m_parseTracksLock);
+ Optional<OSStatus> m_parseTracksStatus WTF_GUARDED_BY_LOCK(m_parseTracksLock);
+ Vector<Ref<MediaTrackReader>> m_trackReaders WTF_GUARDED_BY_LOCK(m_parseTracksLock);
RefPtr<const Logger> m_logger;
const void* m_logIdentifier;
};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
