Modified: trunk/LayoutTests/ChangeLog (245801 => 245802)
--- trunk/LayoutTests/ChangeLog 2019-05-27 22:00:26 UTC (rev 245801)
+++ trunk/LayoutTests/ChangeLog 2019-05-28 00:10:41 UTC (rev 245802)
@@ -1,3 +1,13 @@
+2019-05-27 Takashi Komori <[email protected]>
+
+ [CURL] Fix crashing SocketStreamHandle.
+ https://bugs.webkit.org/show_bug.cgi?id=197873
+
+ Reviewed by Fujii Hironori.
+
+ * platform/wincairo-wk1/TestExpectations:
+ * platform/wincairo/TestExpectations:
+
2019-05-27 Oriol Brufau <[email protected]>
[css-grid] Preserve repeat() notation when serializing declared values
Modified: trunk/LayoutTests/platform/wincairo/TestExpectations (245801 => 245802)
--- trunk/LayoutTests/platform/wincairo/TestExpectations 2019-05-27 22:00:26 UTC (rev 245801)
+++ trunk/LayoutTests/platform/wincairo/TestExpectations 2019-05-28 00:10:41 UTC (rev 245802)
@@ -976,7 +976,6 @@
http/tests/websocket/tests/hybi/websocket-allowed-setting-cookie-as-third-party.html [ Pass Failure ]
http/tests/websocket/tests/hybi/websocket-blocked-from-setting-cookie-as-third-party.html [ Pass Failure ]
http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html [ Pass Failure ]
-http/tests/websocket/tests/hybi/workers/close.html [ Pass Failure ]
http/tests/websocket/tests/hybi/workers/worker-reload.html [ Timeout Pass ]
http/tests/workers/service [ Skip ]
Modified: trunk/LayoutTests/platform/wincairo-wk1/TestExpectations (245801 => 245802)
--- trunk/LayoutTests/platform/wincairo-wk1/TestExpectations 2019-05-27 22:00:26 UTC (rev 245801)
+++ trunk/LayoutTests/platform/wincairo-wk1/TestExpectations 2019-05-28 00:10:41 UTC (rev 245802)
@@ -293,6 +293,8 @@
# Failures on WebKit Legacy
+webkit.org/b/89153 http/tests/websocket/tests/hybi/workers/close.html [ Pass Failure ]
+
# Cookie policy only supported in WK2.
http/tests/cookies/only-accept-first-party-cookies.html [ Skip ]
http/tests/cookies/third-party-cookie-relaxing.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (245801 => 245802)
--- trunk/Source/WebCore/ChangeLog 2019-05-27 22:00:26 UTC (rev 245801)
+++ trunk/Source/WebCore/ChangeLog 2019-05-28 00:10:41 UTC (rev 245802)
@@ -1,3 +1,24 @@
+2019-05-27 Takashi Komori <[email protected]>
+
+ [CURL] Fix crashing SocketStreamHandle.
+ https://bugs.webkit.org/show_bug.cgi?id=197873
+
+ Reviewed by Fujii Hironori.
+
+ When NetworkSocketStream was destructed SocketStreamHandleImple::platformClose was called wrongly times.
+ This is because closed state is not set.
+
+ Test: http/tests/websocket/tests/hybi/workers/close.html
+
+ * platform/network/curl/SocketStreamHandleImpl.h:
+ * platform/network/curl/SocketStreamHandleImplCurl.cpp:
+ (WebCore::SocketStreamHandleImpl::platformSendInternal):
+ (WebCore::SocketStreamHandleImpl::platformClose):
+ (WebCore::SocketStreamHandleImpl::threadEntryPoint):
+ (WebCore::SocketStreamHandleImpl::handleError):
+ (WebCore::SocketStreamHandleImpl::callOnWorkerThread):
+ (WebCore::SocketStreamHandleImpl::executeTasks):
+
2019-05-27 Oriol Brufau <[email protected]>
[css-grid] Preserve repeat() notation when serializing declared values
Modified: trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImpl.h (245801 => 245802)
--- trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImpl.h 2019-05-27 22:00:26 UTC (rev 245801)
+++ trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImpl.h 2019-05-28 00:10:41 UTC (rev 245802)
@@ -34,9 +34,10 @@
#include "CurlContext.h"
#include "SocketStreamHandle.h"
#include <pal/SessionID.h>
+#include <wtf/Function.h>
#include <wtf/Lock.h>
+#include <wtf/MessageQueue.h>
#include <wtf/RefCounted.h>
-#include <wtf/Seconds.h>
#include <wtf/StreamBuffer.h>
#include <wtf/Threading.h>
#include <wtf/UniqueArray.h>
@@ -67,7 +68,9 @@
void handleError(CURLcode);
void stopThread();
- static const size_t kWriteBufferSize = 4 * 1024;
+ void callOnWorkerThread(Function<void()>&&);
+ void executeTasks();
+
static const size_t kReadBufferSize = 4 * 1024;
RefPtr<const StorageSessionProvider> m_storageSessionProvider;
@@ -74,8 +77,11 @@
RefPtr<Thread> m_workerThread;
std::atomic<bool> m_running { true };
- std::atomic<size_t> m_writeBufferSize { 0 };
- size_t m_writeBufferOffset;
+ MessageQueue<Function<void()>> m_taskQueue;
+
+ bool m_hasPendingWriteData { false };
+ size_t m_writeBufferSize { 0 };
+ size_t m_writeBufferOffset { 0 };
UniqueArray<uint8_t> m_writeBuffer;
StreamBuffer<uint8_t, 1024 * 1024> m_buffer;
Modified: trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp (245801 => 245802)
--- trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp 2019-05-27 22:00:26 UTC (rev 245801)
+++ trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp 2019-05-28 00:10:41 UTC (rev 245802)
@@ -40,7 +40,6 @@
#include "SocketStreamError.h"
#include "SocketStreamHandleClient.h"
#include "StorageSessionProvider.h"
-#include <wtf/Lock.h>
#include <wtf/MainThread.h>
#include <wtf/URL.h>
#include <wtf/text/CString.h>
@@ -74,25 +73,21 @@
LOG(Network, "SocketStreamHandle %p platformSend", this);
ASSERT(isMainThread());
- // If there's data waiting, return zero to indicate whole data should be put in a m_buffer.
- // This is thread-safe because state is read in atomic. Also even if the state is cleared by
- // worker thread between load() and evaluation of size, it is okay because invocation of
- // sendPendingData() is serialized in the main thread, so that another call will be happen
- // immediately.
- if (m_writeBufferSize.load())
+ if (m_hasPendingWriteData)
return 0;
- if (length > kWriteBufferSize)
- length = kWriteBufferSize;
+ m_hasPendingWriteData = true;
- // We copy the buffer and then set the state atomically to say there are bytes available.
- // The worker thread will skip reading the buffer if no bytes are available, so it won't
- // access the buffer prematurely
- m_writeBuffer = makeUniqueArray<uint8_t>(length);
- memcpy(m_writeBuffer.get(), data, length);
- m_writeBufferOffset = 0;
- m_writeBufferSize.store(length);
+ auto writeBuffer = makeUniqueArray<uint8_t>(length);
+ memcpy(writeBuffer.get(), data, length);
+ callOnWorkerThread([this, writeBuffer = WTFMove(writeBuffer), writeBufferSize = length]() mutable {
+ ASSERT(!isMainThread());
+ m_writeBuffer = WTFMove(writeBuffer);
+ m_writeBufferSize = writeBufferSize;
+ m_writeBufferOffset = 0;
+ });
+
return length;
}
@@ -103,6 +98,7 @@
if (m_state == Closed)
return;
+ m_state = Closed;
stopThread();
m_client.didCloseSocketStream(*this);
@@ -112,7 +108,7 @@
{
ASSERT(!isMainThread());
- CurlSocketHandle socket { m_url, [this](CURLcode errorCode) {
+ CurlSocketHandle socket { m_url.isolatedCopy(), [this](CURLcode errorCode) {
handleError(errorCode);
}};
@@ -128,25 +124,24 @@
});
while (m_running) {
- auto writeBufferSize = m_writeBufferSize.load();
- auto result = socket.wait(20_ms, writeBufferSize > 0);
+ executeTasks();
+
+ auto result = socket.wait(20_ms, m_writeBuffer.get());
if (!result)
continue;
- // These logic only run when there's data waiting. In that case, m_writeBufferSize won't
- // updated by `platformSendInternal()` running in main thread.
+ // These logic only run when there's data waiting.
if (result->writable && m_running) {
- auto offset = m_writeBufferOffset;
- auto bytesSent = socket.send(m_writeBuffer.get() + offset, writeBufferSize - offset);
- offset += bytesSent;
+ auto bytesSent = socket.send(m_writeBuffer.get() + m_writeBufferOffset, m_writeBufferSize - m_writeBufferOffset);
+ m_writeBufferOffset += bytesSent;
- if (writeBufferSize > offset)
- m_writeBufferOffset = offset;
- else {
+ if (m_writeBufferSize <= m_writeBufferOffset) {
m_writeBuffer = nullptr;
+ m_writeBufferSize = 0;
m_writeBufferOffset = 0;
- m_writeBufferSize.store(0);
+
callOnMainThread([this, protectedThis = makeRef(*this)] {
+ m_hasPendingWriteData = false;
sendPendingData();
});
}
@@ -174,12 +169,17 @@
});
}
}
+
+ m_writeBuffer = nullptr;
}
void SocketStreamHandleImpl::handleError(CURLcode errorCode)
{
m_running = false;
- callOnMainThread([this, protectedThis = makeRef(*this), errorCode, localizedDescription = CurlHandle::errorDescription(errorCode)] {
+ callOnMainThread([this, protectedThis = makeRef(*this), errorCode, localizedDescription = CurlHandle::errorDescription(errorCode).isolatedCopy()] {
+ if (m_state == Closed)
+ return;
+
if (errorCode == CURLE_RECV_ERROR)
m_client.didFailToReceiveSocketStreamData(*this);
else
@@ -199,6 +199,21 @@
}
}
+void SocketStreamHandleImpl::callOnWorkerThread(Function<void()>&& task)
+{
+ ASSERT(isMainThread());
+ m_taskQueue.append(std::make_unique<Function<void()>>(WTFMove(task)));
+}
+
+void SocketStreamHandleImpl::executeTasks()
+{
+ ASSERT(!isMainThread());
+
+ auto tasks = m_taskQueue.takeAllMessages();
+ for (auto& task : tasks)
+ (*task)();
+}
+
} // namespace WebCore
#endif
