[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
aroben at apple.com
aroben at apple.com
Wed Dec 22 11:23:50 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 9d005b9db6c0a751017fc38ca42f15d9fa7f697f
Author: aroben at apple.com <aroben at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jul 21 20:25:05 2010 +0000
Teach CoreIPC how to handle messages that are larger than the pipe's buffer
::GetOverlappedResult and ::ReadFile can fail with ERROR_MORE_DATA
when there is more data available on the pipe than was requested in
the read operation. In those cases, the appropriate response is to
perform another read operation to read the extra data. We now do this.
Also, MSDN says that, because we are doing asynchronous read
operations, we should not pass a pointer to ::ReadFile to find out how
many bytes were read. Instead we should always call
::GetOverlappedResult to find this out. I've changed
Connection::readEventHandler to have a single loop that calls
::GetOverlappedResult and ::ReadFile in alternation, rather than
sometimes calling ::ReadFile multiple times in a row, to satisfy this
requirement.
In order to simplify the logic in this function, I've made us request
only a single byte from the pipe when there are no messages already in
the pipe. (Previously we were requesting 4096 bytes in this case.)
This allows us not to have to consider the case where the received
message is smaller than our read buffer. If we decide that this has a
negative impact on performance, we can of course change it. I've
mitigated this somewhat by using ::PeekNamedMessage to find out the
size of the next message in the pipe (if any), so that we can read it
all in one read operation.
Fixes <http://webkit.org/b/42710> <rdar://problem/8197571> Assertion
in Connection::readEventHandler when launching WebKitTestRunner
Reviewed by Anders Carlsson.
* Platform/CoreIPC/win/ConnectionWin.cpp:
(CoreIPC::Connection::readEventHandler): Put the call to
::GetOverlappedResult in the same loop as ::ReadFile so that we will
call them alternately. If ::GetOverlappedResult fails with
ERROR_MORE_DATA, use ::PeekNamedPipe to determine the size of the rest
of the message, then read it from the pipe. After dispatching the
message, use ::PeekNamedPipe to find out the size of the next message
in the pipe so we can read it all in one operation. If there's no
message in the pipe, we'll request just a single byte of the next
message that becomes available, and Windows will tell us when the rest
of the message is ready. If ::ReadFile fails with ERROR_MORE_DATA it
means there is data available now even though we didn't think there
was any. We go back to the top of the loop in this case and call
::GetOverlappedResult again to retrieve the available data.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@63852 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKit2/ChangeLog b/WebKit2/ChangeLog
index d814c3d..36c11d5 100644
--- a/WebKit2/ChangeLog
+++ b/WebKit2/ChangeLog
@@ -1,3 +1,52 @@
+2010-07-21 Adam Roben <aroben at apple.com>
+
+ Teach CoreIPC how to handle messages that are larger than the pipe's
+ buffer
+
+ ::GetOverlappedResult and ::ReadFile can fail with ERROR_MORE_DATA
+ when there is more data available on the pipe than was requested in
+ the read operation. In those cases, the appropriate response is to
+ perform another read operation to read the extra data. We now do this.
+
+ Also, MSDN says that, because we are doing asynchronous read
+ operations, we should not pass a pointer to ::ReadFile to find out how
+ many bytes were read. Instead we should always call
+ ::GetOverlappedResult to find this out. I've changed
+ Connection::readEventHandler to have a single loop that calls
+ ::GetOverlappedResult and ::ReadFile in alternation, rather than
+ sometimes calling ::ReadFile multiple times in a row, to satisfy this
+ requirement.
+
+ In order to simplify the logic in this function, I've made us request
+ only a single byte from the pipe when there are no messages already in
+ the pipe. (Previously we were requesting 4096 bytes in this case.)
+ This allows us not to have to consider the case where the received
+ message is smaller than our read buffer. If we decide that this has a
+ negative impact on performance, we can of course change it. I've
+ mitigated this somewhat by using ::PeekNamedMessage to find out the
+ size of the next message in the pipe (if any), so that we can read it
+ all in one read operation.
+
+ Fixes <http://webkit.org/b/42710> <rdar://problem/8197571> Assertion
+ in Connection::readEventHandler when launching WebKitTestRunner
+
+ Reviewed by Anders Carlsson.
+
+ * Platform/CoreIPC/win/ConnectionWin.cpp:
+ (CoreIPC::Connection::readEventHandler): Put the call to
+ ::GetOverlappedResult in the same loop as ::ReadFile so that we will
+ call them alternately. If ::GetOverlappedResult fails with
+ ERROR_MORE_DATA, use ::PeekNamedPipe to determine the size of the rest
+ of the message, then read it from the pipe. After dispatching the
+ message, use ::PeekNamedPipe to find out the size of the next message
+ in the pipe so we can read it all in one operation. If there's no
+ message in the pipe, we'll request just a single byte of the next
+ message that becomes available, and Windows will tell us when the rest
+ of the message is ready. If ::ReadFile fails with ERROR_MORE_DATA it
+ means there is data available now even though we didn't think there
+ was any. We go back to the top of the loop in this case and call
+ ::GetOverlappedResult again to retrieve the available data.
+
2010-07-21 Sam Weinig <sam at webkit.org>
Reviewed by Anders Carlsson.
diff --git a/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp b/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
index 7296a50..fd7da18 100644
--- a/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
+++ b/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
@@ -108,9 +108,9 @@ void Connection::readEventHandler()
sendOutgoingMessages();
}
- DWORD numberOfBytesRead = 0;
- if (wasConnected) {
+ while (true) {
// Check if we got some data.
+ DWORD numberOfBytesRead = 0;
if (!::GetOverlappedResult(m_connectionPipe, &m_readState, &numberOfBytesRead, FALSE)) {
DWORD error = ::GetLastError();
@@ -118,6 +118,30 @@ void Connection::readEventHandler()
case ERROR_BROKEN_PIPE:
connectionDidClose();
return;
+ case ERROR_MORE_DATA: {
+ // Read the rest of the message out of the pipe.
+
+ DWORD bytesToRead = 0;
+ if (!::PeekNamedPipe(m_connectionPipe, 0, 0, 0, 0, &bytesToRead)) {
+ DWORD error = ::GetLastError();
+ ASSERT_NOT_REACHED();
+ return;
+ }
+
+ // ::GetOverlappedResult told us there's more data. ::PeekNamedPipe shouldn't
+ // contradict it!
+ ASSERT(bytesToRead);
+ if (!bytesToRead)
+ break;
+
+ m_readBuffer.grow(m_readBuffer.size() + bytesToRead);
+ if (!::ReadFile(m_connectionPipe, m_readBuffer.data() + numberOfBytesRead, bytesToRead, 0, &m_readState)) {
+ DWORD error = ::GetLastError();
+ ASSERT_NOT_REACHED();
+ return;
+ }
+ continue;
+ }
// FIXME: We should figure out why we're getting this error.
case ERROR_IO_INCOMPLETE:
@@ -126,39 +150,69 @@ void Connection::readEventHandler()
ASSERT_NOT_REACHED();
}
}
- }
- while (true) {
- if (numberOfBytesRead) {
+ if (!m_readBuffer.isEmpty()) {
// We have a message, let's dispatch it.
// The messageID is encoded at the end of the buffer.
- size_t realBufferSize = numberOfBytesRead - sizeof(MessageID);
+ // Note that we assume here that the message is the same size as m_readBuffer. We can
+ // assume this because we always size m_readBuffer to exactly match the size of the message,
+ // either when receiving ERROR_MORE_DATA from ::GetOverlappedResult above or when
+ // ::PeekNamedPipe tells us the size below. We never set m_readBuffer to a size larger
+ // than the message.
+ size_t realBufferSize = m_readBuffer.size() - sizeof(MessageID);
unsigned messageID = *reinterpret_cast<unsigned*>(m_readBuffer.data() + realBufferSize);
processIncomingMessage(MessageID::fromInt(messageID), adoptPtr(new ArgumentDecoder(m_readBuffer.data(), realBufferSize)));
}
- // FIXME: Do this somewhere else.
- m_readBuffer.resize(inlineMessageMaxSize);
+ // Find out the size of the next message in the pipe (if there is one) so that we can read
+ // it all in one operation. (This is just an optimization to avoid an extra pass through the
+ // loop (if we chose a buffer size that was too small) or allocating extra memory (if we
+ // chose a buffer size that was too large).)
+ DWORD bytesToRead = 0;
+ if (!::PeekNamedPipe(m_connectionPipe, 0, 0, 0, 0, &bytesToRead)) {
+ DWORD error = ::GetLastError();
+ ASSERT_NOT_REACHED();
+ }
+ if (!bytesToRead) {
+ // There's no message waiting in the pipe. Schedule a read of the first byte of the
+ // next message. We'll find out the message's actual size when it arrives. (If we
+ // change this to read more than a single byte for performance reasons, we'll have to
+ // deal with m_readBuffer potentially being larger than the message we read after
+ // calling ::GetOverlappedResult above.)
+ bytesToRead = 1;
+ }
- // Read from the pipe.
- BOOL result = ::ReadFile(m_connectionPipe, m_readBuffer.data(), m_readBuffer.size(), &numberOfBytesRead, &m_readState);
+ m_readBuffer.resize(bytesToRead);
- if (!result) {
- DWORD error = ::GetLastError();
+ // Either read the next available message (which should occur synchronously), or start an
+ // asynchronous read of the next message that becomes available.
+ BOOL result = ::ReadFile(m_connectionPipe, m_readBuffer.data(), m_readBuffer.size(), 0, &m_readState);
+ if (result) {
+ // There was already a message waiting in the pipe, and we read it synchronously.
+ // Process it.
+ continue;
+ }
- if (error == ERROR_IO_PENDING) {
- // There's pending data - readEventHandler will be called again once the read is complete.
- return;
- }
+ DWORD error = ::GetLastError();
- // FIXME: We need to handle other errors here.
- ASSERT_NOT_REACHED();
+ if (error == ERROR_IO_PENDING) {
+ // There are no messages in the pipe currently. readEventHandler will be called again once there is a message.
+ return;
+ }
+
+ if (error == ERROR_MORE_DATA) {
+ // Either a message is available when we didn't think one was, or the message is larger
+ // than ::PeekNamedPipe told us. The former seems far more likely. Probably the message
+ // became available between our calls to ::PeekNamedPipe and ::ReadFile above. Go back
+ // to the top of the loop to use ::GetOverlappedResult to retrieve the available data.
+ continue;
}
- ASSERT(numberOfBytesRead);
+ // FIXME: We need to handle other errors here.
+ ASSERT_NOT_REACHED();
}
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list