Title: [257909] trunk/Source/WebKit
- Revision
- 257909
- Author
- carlo...@webkit.org
- Date
- 2020-03-05 01:11:44 -0800 (Thu, 05 Mar 2020)
Log Message
REGRESSION(r257667): [UNIX] Tests http/tests/incremental/split-hex-entities.pl and http/tests/misc/large-js-program.php are crashing
https://bugs.webkit.org/show_bug.cgi?id=208571
Reviewed by Alex Christensen.
We get a release assert in Connection::processMessage() when trying to get a file descriptor from
m_fileDescriptors array that is empty. The problem is that since r257667, a shared buffer is always used by the
network process to send data to the web process (NetworkResourceLoader::sendBuffer) and shared buffer
encoding/decoding was changed to always use shared memory and send the file descriptor over the IPC. When
sending large data in small chunks like these tests are doing, we easily end up with many messages queued in the
web process receiver (Connection::enqueueIncomingMessage), all of them having one file descriptor open. When the
maximum number of open file descriptors per process is reached, recvmsg doesn't fail but it sets the flag
MSG_CTRUNC in msg_flags and the file descriptor is not actually included as part of the control message. The
message info still claims to include a file descriptor, but it hasn't been created and added to the
m_fileDescriptors array. We could check msg_flags, but only to assert earlier, not to fix the problem, since we
are unable to get the file descriptor sent. So, at least in linux I think it's better to send the data over the
IPC instead of using shared memory. We are already using shared memory for any IPC message bigger than 4096.
* Platform/IPC/unix/ConnectionUnix.cpp:
(IPC::readBytesFromSocket): Consider also a read failure when control data is discarded.
* Shared/WebCoreArgumentCoders.cpp:
(IPC::encodeSharedBuffer): Do not use shared memory to encode a SharedBuffer in Unix.
(IPC::decodeSharedBuffer): Do not use shared memory to decode a SharedBuffer in Unix.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (257908 => 257909)
--- trunk/Source/WebKit/ChangeLog 2020-03-05 08:54:49 UTC (rev 257908)
+++ trunk/Source/WebKit/ChangeLog 2020-03-05 09:11:44 UTC (rev 257909)
@@ -1,3 +1,29 @@
+2020-03-05 Carlos Garcia Campos <cgar...@igalia.com>
+
+ REGRESSION(r257667): [UNIX] Tests http/tests/incremental/split-hex-entities.pl and http/tests/misc/large-js-program.php are crashing
+ https://bugs.webkit.org/show_bug.cgi?id=208571
+
+ Reviewed by Alex Christensen.
+
+ We get a release assert in Connection::processMessage() when trying to get a file descriptor from
+ m_fileDescriptors array that is empty. The problem is that since r257667, a shared buffer is always used by the
+ network process to send data to the web process (NetworkResourceLoader::sendBuffer) and shared buffer
+ encoding/decoding was changed to always use shared memory and send the file descriptor over the IPC. When
+ sending large data in small chunks like these tests are doing, we easily end up with many messages queued in the
+ web process receiver (Connection::enqueueIncomingMessage), all of them having one file descriptor open. When the
+ maximum number of open file descriptors per process is reached, recvmsg doesn't fail but it sets the flag
+ MSG_CTRUNC in msg_flags and the file descriptor is not actually included as part of the control message. The
+ message info still claims to include a file descriptor, but it hasn't been created and added to the
+ m_fileDescriptors array. We could check msg_flags, but only to assert earlier, not to fix the problem, since we
+ are unable to get the file descriptor sent. So, at least in linux I think it's better to send the data over the
+ IPC instead of using shared memory. We are already using shared memory for any IPC message bigger than 4096.
+
+ * Platform/IPC/unix/ConnectionUnix.cpp:
+ (IPC::readBytesFromSocket): Consider also a read failure when control data is discarded.
+ * Shared/WebCoreArgumentCoders.cpp:
+ (IPC::encodeSharedBuffer): Do not use shared memory to encode a SharedBuffer in Unix.
+ (IPC::decodeSharedBuffer): Do not use shared memory to decode a SharedBuffer in Unix.
+
2020-03-04 Chris Dumez <cdu...@apple.com>
WebsiteDataStore methods often create process pools and launch network processes unnecessarily
Modified: trunk/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp (257908 => 257909)
--- trunk/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp 2020-03-05 08:54:49 UTC (rev 257908)
+++ trunk/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp 2020-03-05 09:11:44 UTC (rev 257909)
@@ -270,6 +270,12 @@
return -1;
}
+ if (message.msg_flags & MSG_CTRUNC) {
+ // Control data has been discarded, which is expected by processMessage(), so consider this a read failure.
+ buffer.shrink(previousBufferSize);
+ return -1;
+ }
+
struct cmsghdr* controlMessage;
for (controlMessage = CMSG_FIRSTHDR(&message); controlMessage; controlMessage = CMSG_NXTHDR(&message, controlMessage)) {
if (controlMessage->cmsg_level == SOL_SOCKET && controlMessage->cmsg_type == SCM_RIGHTS) {
Modified: trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp (257908 => 257909)
--- trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp 2020-03-05 08:54:49 UTC (rev 257908)
+++ trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp 2020-03-05 09:11:44 UTC (rev 257909)
@@ -122,16 +122,25 @@
static void encodeSharedBuffer(Encoder& encoder, const SharedBuffer* buffer)
{
- SharedMemory::Handle handle;
uint64_t bufferSize = buffer ? buffer->size() : 0;
encoder << bufferSize;
if (!bufferSize)
return;
+#if USE(UNIX_DOMAIN_SOCKETS)
+ // Do not use shared memory for SharedBuffer encoding in Unix, because it's easy to reach the
+ // maximum number of file descriptors open per process when sending large data in small chunks
+ // over the IPC. ConnectionUnix.cpp already uses shared memory to send any IPC message that is
+ // too large. See https://bugs.webkit.org/show_bug.cgi?id=208571.
+ for (const auto& element : *buffer)
+ encoder.encodeFixedLengthData(reinterpret_cast<const uint8_t*>(element.segment->data()), element.segment->size(), 1);
+#else
+ SharedMemory::Handle handle;
auto sharedMemoryBuffer = SharedMemory::allocate(buffer->size());
memcpy(sharedMemoryBuffer->data(), buffer->data(), buffer->size());
sharedMemoryBuffer->createHandle(handle, SharedMemory::Protection::ReadOnly);
encoder << handle;
+#endif
}
static bool decodeSharedBuffer(Decoder& decoder, RefPtr<SharedBuffer>& buffer)
@@ -143,6 +152,14 @@
if (!bufferSize)
return true;
+#if USE(UNIX_DOMAIN_SOCKETS)
+ Vector<uint8_t> data;
+ data.grow(bufferSize);
+ if (!decoder.decodeFixedLengthData(data.data(), data.size(), 1))
+ return false;
+
+ buffer = SharedBuffer::create(WTFMove(data));
+#else
SharedMemory::Handle handle;
if (!decoder.decode(handle))
return false;
@@ -149,6 +166,7 @@
auto sharedMemoryBuffer = SharedMemory::map(handle, SharedMemory::Protection::ReadOnly);
buffer = SharedBuffer::create(static_cast<unsigned char*>(sharedMemoryBuffer->data()), bufferSize);
+#endif
return true;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
