Changeset 64223 in webkit
- Timestamp:
- Jul 28, 2010 12:54:13 PM (14 years ago)
- Location:
- trunk/WebKit2
- Files:
-
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/WebKit2/ChangeLog
r64221 r64223 1 2010-07-28 Adam Roben <aroben@apple.com> 2 3 Teach CoreIPC the right way to send large messages on Windows 4 5 r63776 added support for ::WriteFile failing with ERROR_IO_PENDING, 6 but it had a major flaw: we didn't ensure that the data being sent 7 (which is owned by the ArgumentEncoder) stayed around until the write 8 finished. We'd destroy the data immediately, leading to ::WriteFile 9 accessing that freed memory later. This seemed to always manifest 10 itself as a crash in ::WaitForMultipleObjects. 11 12 The correct solution (as hinted above) is to make sure that the data 13 being written is not destroyed until the write completes. When 14 ::WriteFile fails with ERROR_IO_PENDING, we store the data being sent 15 in Connection::m_pendingWriteArguments, and don't send any more 16 messages until that write completes. We use an event in the OVERLAPPED 17 structure passed to ::WriteFile to detect when the write has completed 18 (similar to what we do for reads). 19 20 Fixes <http://webkit.org/b/42785> <rdar://problem/8218522> Crash in 21 WebKit2WebProcess in WaitForMultipleObjects beneath 22 WorkQueue::workQueueThreadBody when running tests that produce a lot 23 of output 24 25 Reviewed by Anders Carlsson. 26 27 * Platform/CoreIPC/Connection.cpp: 28 (CoreIPC::Connection::canSendOutgoingMessages): Added. This calls out 29 to a platform-specific function to allow each platform to have its own 30 policy for when messages can and can't be sent. 31 (CoreIPC::Connection::sendOutgoingMessages): Use the new 32 canSendOutgoingMessages to determine whether we can send any messages 33 right now. We now remove one message at a time from m_outgoingMessages 34 and send it. We stop sending messages when sendOutgoingMessage returns 35 false. 36 37 * Platform/CoreIPC/Connection.h: Added m_pendingWriteArguments and 38 m_writeState on Windows. 39 (CoreIPC::Connection::Message::Message): Added this default 40 constructor. 41 42 * Platform/CoreIPC/MessageID.h: 43 (CoreIPC::MessageID::MessageID): Made the default constructor public 44 for Message's benefit. 45 46 * Platform/CoreIPC/mac/ConnectionMac.cpp: 47 (CoreIPC::Connection::platformCanSendOutgoingMessages): Added. Always 48 returns true. 49 (CoreIPC::Connection::sendOutgoingMessage): Changed to return a 50 boolean indicating whether more messages can be sent at this time. 51 52 * Platform/CoreIPC/qt/ConnectionQt.cpp: 53 (CoreIPC::Connection::platformCanSendOutgoingMessages): Added. Returns 54 true if we have a socket. 55 (CoreIPC::Connection::sendOutgoingMessage): Changed a null-check of 56 m_socket to an assertion since it should be checked for null in 57 platformCanSendOutgoingMessages. Changed to return a boolean 58 indicating whether more messages can be sent at this time. 59 60 * Platform/CoreIPC/win/ConnectionWin.cpp: 61 (CoreIPC::Connection::platformInitialize): Added initialization of 62 m_writeState. 63 (CoreIPC::Connection::platformInvalidate): Close m_writeState's event 64 handle. 65 (CoreIPC::Connection::writeEventHandler): Added. Checks if the pending 66 write has completed, cleans up our pending write state, and sends any 67 remaining messages. 68 (CoreIPC::Connection::open): Register our write event with the 69 WorkQueue so that writeEventHandler will be called when the event is 70 signaled. 71 (CoreIPC::Connection::platformCanSendOutgoingMessages): Added. We can 72 only send messages if there isn't a write pending. 73 (CoreIPC::Connection::sendOutgoingMessage): Changed to return a 74 boolean indicating whether more messages can be sent at this time. We 75 now pass m_writeState to ::WriteFile instead of an empty OVERLAPPED 76 struct so that our write event will be signaled when the write 77 completes. We also no longer pass a pointer to receive how many bytes 78 were written, as recommended by MSDN. If ::WriteFile fails with 79 ERROR_IO_PENDING, we save the ArgumentEncoder for this message and 80 return false to indicate that no more messages can be sent at this 81 time. 82 1 83 2010-07-28 Adam Roben <aroben@apple.com> 2 84 -
trunk/WebKit2/Platform/CoreIPC/Connection.cpp
r63916 r64223 208 208 } 209 209 210 bool Connection::canSendOutgoingMessages() const 211 { 212 return m_isConnected && platformCanSendOutgoingMessages(); 213 } 214 210 215 void Connection::sendOutgoingMessages() 211 216 { 212 if (! m_isConnected)217 if (!canSendOutgoingMessages()) 213 218 return; 214 219 215 Vector<OutgoingMessage> outgoingMessages; 216 217 { 218 MutexLocker locker(m_outgoingMessagesLock); 219 m_outgoingMessages.swap(outgoingMessages); 220 } 221 222 // Send messages. 223 for (size_t i = 0; i < outgoingMessages.size(); ++i) 224 sendOutgoingMessage(outgoingMessages[i].messageID(), adoptPtr(outgoingMessages[i].arguments())); 220 while (true) { 221 OutgoingMessage message; 222 { 223 MutexLocker locker(m_outgoingMessagesLock); 224 if (m_outgoingMessages.isEmpty()) 225 break; 226 message = m_outgoingMessages.takeFirst(); 227 } 228 229 if (!sendOutgoingMessage(message.messageID(), adoptPtr(message.arguments()))) 230 break; 231 } 225 232 } 226 233 -
trunk/WebKit2/Platform/CoreIPC/Connection.h
r63148 r64223 93 93 template<typename T> class Message { 94 94 public: 95 Message() 96 : m_arguments(0) 97 { 98 } 99 95 100 Message(MessageID messageID, PassOwnPtr<T> arguments) 96 101 : m_messageID(messageID) … … 127 132 // Called on the connection work queue. 128 133 void processIncomingMessage(MessageID, PassOwnPtr<ArgumentDecoder>); 134 bool canSendOutgoingMessages() const; 135 bool platformCanSendOutgoingMessages() const; 129 136 void sendOutgoingMessages(); 130 voidsendOutgoingMessage(MessageID, PassOwnPtr<ArgumentEncoder>);137 bool sendOutgoingMessage(MessageID, PassOwnPtr<ArgumentEncoder>); 131 138 void connectionDidClose(); 132 139 … … 151 158 // Outgoing messages. 152 159 Mutex m_outgoingMessagesLock; 153 Vector<OutgoingMessage> m_outgoingMessages;160 Deque<OutgoingMessage> m_outgoingMessages; 154 161 155 162 ThreadCondition m_waitForMessageCondition; … … 167 174 // Called on the connection queue. 168 175 void readEventHandler(); 176 void writeEventHandler(); 169 177 170 178 Vector<uint8_t> m_readBuffer; 171 179 OVERLAPPED m_readState; 180 OwnPtr<ArgumentEncoder> m_pendingWriteArguments; 181 OVERLAPPED m_writeState; 172 182 HANDLE m_connectionPipe; 173 183 #elif PLATFORM(QT) -
trunk/WebKit2/Platform/CoreIPC/MessageID.h
r61718 r64223 68 68 }; 69 69 70 MessageID() 71 : m_messageID(0) 72 { 73 } 74 70 75 template <typename EnumType> 71 76 explicit MessageID(EnumType messageKind, unsigned char flags = 0) … … 114 119 unsigned char getClass() const { return (m_messageID & 0x00ff0000) >> 16; } 115 120 116 MessageID()117 : m_messageID(0)118 {119 }120 121 121 unsigned m_messageID; 122 122 }; -
trunk/WebKit2/Platform/CoreIPC/mac/ConnectionMac.cpp
r63708 r64223 116 116 } 117 117 118 void Connection::sendOutgoingMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> arguments) 118 bool Connection::platformCanSendOutgoingMessages() const 119 { 120 return true; 121 } 122 123 bool Connection::sendOutgoingMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> arguments) 119 124 { 120 125 Vector<Attachment> attachments = arguments->releaseAttachments(); … … 200 205 if (kr != KERN_SUCCESS) { 201 206 // FIXME: What should we do here? 202 return; 203 } 207 } 208 209 return true; 204 210 } 205 211 -
trunk/WebKit2/Platform/CoreIPC/qt/ConnectionQt.cpp
r63148 r64223 105 105 } 106 106 107 void Connection::sendOutgoingMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> arguments) 107 bool Connection::platformCanSendOutgoingMessages() const 108 108 { 109 if (!m_socket) 110 return; 109 return m_socket; 110 } 111 112 bool Connection::sendOutgoingMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> arguments) 113 { 114 ASSERT(m_socket); 111 115 112 116 // We put the message ID last. … … 122 126 123 127 ASSERT(bytesWritten == arguments->bufferSize()); 128 return true; 124 129 } 125 130 -
trunk/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
r64221 r64223 83 83 { 84 84 memset(&m_readState, 0, sizeof(m_readState)); 85 m_readState.hEvent = ::CreateEventW(0, false, false, 0); 85 m_readState.hEvent = ::CreateEventW(0, FALSE, FALSE, 0); 86 87 memset(&m_writeState, 0, sizeof(m_writeState)); 88 m_writeState.hEvent = ::CreateEventW(0, FALSE, FALSE, 0); 86 89 87 90 m_connectionPipe = identifier; … … 97 100 ::CloseHandle(m_readState.hEvent); 98 101 m_readState.hEvent = 0; 102 103 ::CloseHandle(m_writeState.hEvent); 104 m_writeState.hEvent = 0; 99 105 100 106 ::CloseHandle(m_connectionPipe); … … 228 234 } 229 235 236 void Connection::writeEventHandler() 237 { 238 DWORD numberOfBytesWritten = 0; 239 if (!::GetOverlappedResult(m_connectionPipe, &m_writeState, &numberOfBytesWritten, FALSE)) { 240 DWORD error = ::GetLastError(); 241 ASSERT_NOT_REACHED(); 242 } 243 244 // The pending write has finished, so we are now done with its arguments. Clearing this member 245 // will allow us to send messages again. 246 m_pendingWriteArguments = 0; 247 248 // Now that the pending write has finished, we can try to send a new message. 249 sendOutgoingMessages(); 250 } 251 230 252 bool Connection::open() 231 253 { 232 // Start listening for read state events.254 // Start listening for read and write state events. 233 255 m_connectionQueue.registerHandle(m_readState.hEvent, WorkItem::create(this, &Connection::readEventHandler)); 256 m_connectionQueue.registerHandle(m_writeState.hEvent, WorkItem::create(this, &Connection::writeEventHandler)); 234 257 235 258 if (m_isServer) { … … 262 285 } 263 286 264 void Connection::sendOutgoingMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> arguments) 265 { 287 bool Connection::platformCanSendOutgoingMessages() const 288 { 289 // We only allow sending one asynchronous message at a time. If we wanted to send more than one 290 // at once, we'd have to use multiple OVERLAPPED structures and hold onto multiple pending 291 // ArgumentEncoders (one of each for each simultaneous asynchronous message). 292 return !m_pendingWriteArguments; 293 } 294 295 bool Connection::sendOutgoingMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> arguments) 296 { 297 ASSERT(!m_pendingWriteArguments); 298 266 299 // Just bail if the handle has been closed. 267 300 if (m_connectionPipe == INVALID_HANDLE_VALUE) 268 return ;301 return false; 269 302 270 303 // We put the message ID last. … … 272 305 273 306 // Write the outgoing message. 274 OVERLAPPED overlapped = { 0 }; 275 276 DWORD bytesWritten; 277 if (::WriteFile(m_connectionPipe, arguments->buffer(), arguments->bufferSize(), &bytesWritten, &overlapped)) { 307 308 if (::WriteFile(m_connectionPipe, arguments->buffer(), arguments->bufferSize(), 0, &m_writeState)) { 278 309 // We successfully sent this message. 279 return ;310 return true; 280 311 } 281 312 282 313 DWORD error = ::GetLastError(); 283 if (error == ERROR_IO_PENDING) { 284 // The message will be sent soon. 285 return; 286 } 287 288 ASSERT_NOT_REACHED(); 314 if (error != ERROR_IO_PENDING) { 315 ASSERT_NOT_REACHED(); 316 return false; 317 } 318 319 // The message will be sent soon. Hold onto the arguments so that they won't be destroyed 320 // before the write completes. 321 m_pendingWriteArguments = arguments; 322 323 // We can only send one asynchronous message at a time (see comment in platformCanSendOutgoingMessages). 324 return false; 289 325 } 290 326
Note: See TracChangeset
for help on using the changeset viewer.