- Revision
- 280935
- Author
- [email protected]
- Date
- 2021-08-11 15:33:17 -0700 (Wed, 11 Aug 2021)
Log Message
ThreadSanitizer: data race in WTF::StringImpl::deref() under WebKit::NetworkCache::IOChannel::~IOChannel()
<https://webkit.org/b/229003>
<rdar://problem/81795626>
Reviewed by Chris Dumez.
Covered by 3245 layout tests running with TSan including:
http/wpt/service-workers/file-upload.html
* NetworkProcess/cache/NetworkCacheIOChannel.h:
(WebKit::NetworkCache::IOChannel::open):
- Update to use #pragma once.
- Make an isolatedCopy() for m_path.
(WebKit::NetworkCache::IOChannel::IOChannel):
- Switch to using an rvalue reference.
* NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm:
(WebKit::NetworkCache::IOChannel::IOChannel): Ditto.
* NetworkProcess/cache/NetworkCacheIOChannelCurl.cpp:
(WebKit::NetworkCache::IOChannel::IOChannel): Ditto.
* NetworkProcess/cache/NetworkCacheIOChannelGLib.cpp:
(WebKit::NetworkCache::IOChannel::IOChannel): Ditto.
- Switch to use m_path instead of filePath to prevent
use-after-move.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (280934 => 280935)
--- trunk/Source/WebKit/ChangeLog 2021-08-11 22:25:56 UTC (rev 280934)
+++ trunk/Source/WebKit/ChangeLog 2021-08-11 22:33:17 UTC (rev 280935)
@@ -1,3 +1,29 @@
+2021-08-11 David Kilzer <[email protected]>
+
+ ThreadSanitizer: data race in WTF::StringImpl::deref() under WebKit::NetworkCache::IOChannel::~IOChannel()
+ <https://webkit.org/b/229003>
+ <rdar://problem/81795626>
+
+ Reviewed by Chris Dumez.
+
+ Covered by 3245 layout tests running with TSan including:
+ http/wpt/service-workers/file-upload.html
+
+ * NetworkProcess/cache/NetworkCacheIOChannel.h:
+ (WebKit::NetworkCache::IOChannel::open):
+ - Update to use #pragma once.
+ - Make an isolatedCopy() for m_path.
+ (WebKit::NetworkCache::IOChannel::IOChannel):
+ - Switch to using an rvalue reference.
+ * NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm:
+ (WebKit::NetworkCache::IOChannel::IOChannel): Ditto.
+ * NetworkProcess/cache/NetworkCacheIOChannelCurl.cpp:
+ (WebKit::NetworkCache::IOChannel::IOChannel): Ditto.
+ * NetworkProcess/cache/NetworkCacheIOChannelGLib.cpp:
+ (WebKit::NetworkCache::IOChannel::IOChannel): Ditto.
+ - Switch to use m_path instead of filePath to prevent
+ use-after-move.
+
2021-08-11 Sihui Liu <[email protected]>
Suspend WorkQueue of ResourceLoadStatistics and LocalStorage sooner
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannel.h (280934 => 280935)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannel.h 2021-08-11 22:25:56 UTC (rev 280934)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannel.h 2021-08-11 22:33:17 UTC (rev 280935)
@@ -23,8 +23,7 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/
-#ifndef NetworkCacheIOChannel_h
-#define NetworkCacheIOChannel_h
+#pragma once
#include "NetworkCacheData.h"
#include <wtf/Function.h>
@@ -46,7 +45,7 @@
class IOChannel : public ThreadSafeRefCounted<IOChannel> {
public:
enum class Type { Read, Write, Create };
- static Ref<IOChannel> open(const String& file, Type type, std::optional<WorkQueue::QOS> qos = { }) { return adoptRef(*new IOChannel(file, type, qos)); }
+ static Ref<IOChannel> open(const String& file, Type type, std::optional<WorkQueue::QOS> qos = { }) { return adoptRef(*new IOChannel(file.isolatedCopy(), type, qos)); }
// Using nullptr as queue submits the result to the main queue.
// FIXME: We should add WorkQueue::main() instead.
@@ -65,7 +64,7 @@
~IOChannel();
private:
- IOChannel(const String& filePath, IOChannel::Type, std::optional<WorkQueue::QOS>);
+ IOChannel(String&& filePath, IOChannel::Type, std::optional<WorkQueue::QOS>);
#if USE(GLIB)
void readSyncInThread(size_t offset, size_t, WorkQueue&, Function<void (Data&, int error)>&&);
@@ -88,7 +87,5 @@
#endif
};
-}
-}
-
-#endif
+} // namespace NetworkCache
+} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm (280934 => 280935)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm 2021-08-11 22:25:56 UTC (rev 280934)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm 2021-08-11 22:33:17 UTC (rev 280935)
@@ -51,11 +51,11 @@
}
}
-IOChannel::IOChannel(const String& filePath, Type type, std::optional<WorkQueue::QOS> qos)
- : m_path(filePath)
+IOChannel::IOChannel(String&& filePath, Type type, std::optional<WorkQueue::QOS> qos)
+ : m_path(WTFMove(filePath))
, m_type(type)
{
- auto path = FileSystem::fileSystemRepresentation(filePath);
+ auto path = FileSystem::fileSystemRepresentation(m_path);
int oflag;
mode_t mode;
WorkQueue::QOS dispatchQOS;
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCurl.cpp (280934 => 280935)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCurl.cpp 2021-08-11 22:25:56 UTC (rev 280934)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCurl.cpp 2021-08-11 22:33:17 UTC (rev 280935)
@@ -31,8 +31,8 @@
namespace WebKit {
namespace NetworkCache {
-IOChannel::IOChannel(const String& filePath, Type type, std::optional<WorkQueue::QOS>)
- : m_path(filePath)
+IOChannel::IOChannel(String&& filePath, Type type, std::optional<WorkQueue::QOS>)
+ : m_path(WTFMove(filePath))
, m_type(type)
{
FileSystem::FileOpenMode mode { };
@@ -47,7 +47,7 @@
mode = FileSystem::FileOpenMode::Write;
break;
}
- m_fileDescriptor = FileSystem::openFile(filePath, mode);
+ m_fileDescriptor = FileSystem::openFile(m_path, mode);
}
IOChannel::~IOChannel()
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelGLib.cpp (280934 => 280935)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelGLib.cpp 2021-08-11 22:25:56 UTC (rev 280934)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelGLib.cpp 2021-08-11 22:33:17 UTC (rev 280935)
@@ -37,11 +37,11 @@
static const size_t gDefaultReadBufferSize = 4096;
-IOChannel::IOChannel(const String& filePath, Type type, std::optional<WorkQueue::QOS>)
- : m_path(filePath)
+IOChannel::IOChannel(String&& filePath, Type type, std::optional<WorkQueue::QOS>)
+ : m_path(WTFMove(filePath))
, m_type(type)
{
- auto path = FileSystem::fileSystemRepresentation(filePath);
+ auto path = FileSystem::fileSystemRepresentation(m_path);
GRefPtr<GFile> file = adoptGRef(g_file_new_for_path(path.data()));
switch (m_type) {
case Type::Create: {
