Commit ee63dac3 authored by andersca@apple.com's avatar andersca@apple.com

2011-03-26 Anders Carlsson <andersca@apple.com>

        Reviewed by Sam Weinig.

        ASSERTION FAILED: m_operationInProgress == NoOperation loading nytimes.com
        https://bugs.webkit.org/show_bug.cgi?id=57165
        <rdar://problem/9024311>

        The assertion fired because during GC, the web process sends a synchronous NPObjectMessageReceiver::Deallocate
        message to the plug-in process. Since this is a synchronous message, the web process needs to process incoming synchronous
        messages. While waiting, we get an incoming PluginProxy::Evaluate message from the plug-in. This causes JavaScript to run
        during GC which is very bad.

        The fix for this is to add a flag on the connection that will cause synchronous messages sent by the connection (in this case the
        plug-in process) to not be processed while the other side (the web process) is waiting for a synchronous reply _unless_ the connection
        is actually processing a synchronous message. (The last part is to avoid deadlocks).

        Since the call to NPN_Evaluate by the plug-in (that ends up sending the PluginProxy::Evaluate message) comes from a run loop timer firing,
        it's OK to wait for it to be processed by the web process when it returns to the run loop.

        * Platform/CoreIPC/Connection.cpp:
        (CoreIPC::Connection::Connection):
        Initialize m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage and m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount.

        (CoreIPC::Connection::setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage):
        Set m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage.

        (CoreIPC::Connection::sendMessage):
        Don't add the MessageID::DispatchMessageWhenWaitingForSyncReply flag when the right flags has been set on the connection, and it's not processing a synchronous message.

        (CoreIPC::Connection::dispatchMessage):
        Increment and decrement m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount accordingly.

        * PluginProcess/WebProcessConnection.cpp:
        (WebKit::WebProcessConnection::WebProcessConnection):
        Call setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage on the connection.


git-svn-id: svn://svn.chromium.org/blink/trunk@82045 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 8fa9d46a
2011-03-26 Anders Carlsson <andersca@apple.com>
Reviewed by Sam Weinig.
ASSERTION FAILED: m_operationInProgress == NoOperation loading nytimes.com
https://bugs.webkit.org/show_bug.cgi?id=57165
<rdar://problem/9024311>
The assertion fired because during GC, the web process sends a synchronous NPObjectMessageReceiver::Deallocate
message to the plug-in process. Since this is a synchronous message, the web process needs to process incoming synchronous
messages. While waiting, we get an incoming PluginProxy::Evaluate message from the plug-in. This causes JavaScript to run
during GC which is very bad.
The fix for this is to add a flag on the connection that will cause synchronous messages sent by the connection (in this case the
plug-in process) to not be processed while the other side (the web process) is waiting for a synchronous reply _unless_ the connection
is actually processing a synchronous message. (The last part is to avoid deadlocks).
Since the call to NPN_Evaluate by the plug-in (that ends up sending the PluginProxy::Evaluate message) comes from a run loop timer firing,
it's OK to wait for it to be processed by the web process when it returns to the run loop.
* Platform/CoreIPC/Connection.cpp:
(CoreIPC::Connection::Connection):
Initialize m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage and m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount.
(CoreIPC::Connection::setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage):
Set m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage.
(CoreIPC::Connection::sendMessage):
Don't add the MessageID::DispatchMessageWhenWaitingForSyncReply flag when the right flags has been set on the connection, and it's not processing a synchronous message.
(CoreIPC::Connection::dispatchMessage):
Increment and decrement m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount accordingly.
* PluginProcess/WebProcessConnection.cpp:
(WebKit::WebProcessConnection::WebProcessConnection):
Call setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage on the connection.
2011-03-26 Sam Weinig <sam@webkit.org> 2011-03-26 Sam Weinig <sam@webkit.org>
Rollout r82042 (If a user doesn't have a Database/LocalStorage directory, it can't be created (sandbox violations)) Rollout r82042 (If a user doesn't have a Database/LocalStorage directory, it can't be created (sandbox violations))
......
...@@ -188,11 +188,13 @@ Connection::Connection(Identifier identifier, bool isServer, Client* client, Run ...@@ -188,11 +188,13 @@ Connection::Connection(Identifier identifier, bool isServer, Client* client, Run
: m_client(client) : m_client(client)
, m_isServer(isServer) , m_isServer(isServer)
, m_syncRequestID(0) , m_syncRequestID(0)
, m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(false)
, m_didCloseOnConnectionWorkQueueCallback(0) , m_didCloseOnConnectionWorkQueueCallback(0)
, m_isConnected(false) , m_isConnected(false)
, m_connectionQueue("com.apple.CoreIPC.ReceiveQueue") , m_connectionQueue("com.apple.CoreIPC.ReceiveQueue")
, m_clientRunLoop(clientRunLoop) , m_clientRunLoop(clientRunLoop)
, m_inDispatchMessageCount(0) , m_inDispatchMessageCount(0)
, m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount(0)
, m_didReceiveInvalidMessage(false) , m_didReceiveInvalidMessage(false)
, m_syncMessageState(SyncMessageState::getOrCreate(clientRunLoop)) , m_syncMessageState(SyncMessageState::getOrCreate(clientRunLoop))
, m_shouldWaitForSyncReplies(true) , m_shouldWaitForSyncReplies(true)
...@@ -209,6 +211,13 @@ Connection::~Connection() ...@@ -209,6 +211,13 @@ Connection::~Connection()
m_connectionQueue.invalidate(); m_connectionQueue.invalidate();
} }
void Connection::setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(bool flag)
{
ASSERT(!m_isConnected);
m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage = flag;
}
void Connection::setDidCloseOnConnectionWorkQueueCallback(DidCloseOnConnectionWorkQueueCallback callback) void Connection::setDidCloseOnConnectionWorkQueueCallback(DidCloseOnConnectionWorkQueueCallback callback)
{ {
ASSERT(!m_isConnected); ASSERT(!m_isConnected);
...@@ -253,7 +262,9 @@ bool Connection::sendMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> ar ...@@ -253,7 +262,9 @@ bool Connection::sendMessage(MessageID messageID, PassOwnPtr<ArgumentEncoder> ar
if (!isValid()) if (!isValid())
return false; return false;
if (messageSendFlags & DispatchMessageEvenWhenWaitingForSyncReply) if (messageSendFlags & DispatchMessageEvenWhenWaitingForSyncReply
&& (!m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage
|| m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount))
messageID = messageID.messageIDWithAddedFlags(MessageID::DispatchMessageWhenWaitingForSyncReply); messageID = messageID.messageIDWithAddedFlags(MessageID::DispatchMessageWhenWaitingForSyncReply);
MutexLocker locker(m_outgoingMessagesLock); MutexLocker locker(m_outgoingMessagesLock);
...@@ -573,6 +584,9 @@ void Connection::dispatchMessage(IncomingMessage& message) ...@@ -573,6 +584,9 @@ void Connection::dispatchMessage(IncomingMessage& message)
m_inDispatchMessageCount++; m_inDispatchMessageCount++;
if (message.messageID().shouldDispatchMessageWhenWaitingForSyncReply())
m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount++;
bool oldDidReceiveInvalidMessage = m_didReceiveInvalidMessage; bool oldDidReceiveInvalidMessage = m_didReceiveInvalidMessage;
m_didReceiveInvalidMessage = false; m_didReceiveInvalidMessage = false;
...@@ -584,6 +598,9 @@ void Connection::dispatchMessage(IncomingMessage& message) ...@@ -584,6 +598,9 @@ void Connection::dispatchMessage(IncomingMessage& message)
m_didReceiveInvalidMessage |= arguments->isInvalid(); m_didReceiveInvalidMessage |= arguments->isInvalid();
m_inDispatchMessageCount--; m_inDispatchMessageCount--;
if (message.messageID().shouldDispatchMessageWhenWaitingForSyncReply())
m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount--;
if (m_didReceiveInvalidMessage && m_client) if (m_didReceiveInvalidMessage && m_client)
m_client->didReceiveInvalidMessage(this, message.messageID()); m_client->didReceiveInvalidMessage(this, message.messageID());
......
...@@ -114,6 +114,8 @@ public: ...@@ -114,6 +114,8 @@ public:
void setShouldCloseConnectionOnProcessTermination(WebKit::PlatformProcessIdentifier); void setShouldCloseConnectionOnProcessTermination(WebKit::PlatformProcessIdentifier);
#endif #endif
void setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(bool);
// The set callback will be called on the connection work queue when the connection is closed, // The set callback will be called on the connection work queue when the connection is closed,
// before didCall is called on the client thread. Must be called before the connection is opened. // before didCall is called on the client thread. Must be called before the connection is opened.
// In the future we might want a more generic way to handle sync or async messages directly // In the future we might want a more generic way to handle sync or async messages directly
...@@ -214,13 +216,15 @@ private: ...@@ -214,13 +216,15 @@ private:
bool m_isServer; bool m_isServer;
uint64_t m_syncRequestID; uint64_t m_syncRequestID;
bool m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage;
DidCloseOnConnectionWorkQueueCallback m_didCloseOnConnectionWorkQueueCallback; DidCloseOnConnectionWorkQueueCallback m_didCloseOnConnectionWorkQueueCallback;
bool m_isConnected; bool m_isConnected;
WorkQueue m_connectionQueue; WorkQueue m_connectionQueue;
RunLoop* m_clientRunLoop; RunLoop* m_clientRunLoop;
uint32_t m_inDispatchMessageCount; unsigned m_inDispatchMessageCount;
unsigned m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount;
bool m_didReceiveInvalidMessage; bool m_didReceiveInvalidMessage;
// Incoming messages. // Incoming messages.
......
...@@ -52,6 +52,7 @@ WebProcessConnection::WebProcessConnection(CoreIPC::Connection::Identifier conne ...@@ -52,6 +52,7 @@ WebProcessConnection::WebProcessConnection(CoreIPC::Connection::Identifier conne
m_connection = CoreIPC::Connection::createServerConnection(connectionIdentifier, this, RunLoop::main()); m_connection = CoreIPC::Connection::createServerConnection(connectionIdentifier, this, RunLoop::main());
m_npRemoteObjectMap = NPRemoteObjectMap::create(m_connection.get()); m_npRemoteObjectMap = NPRemoteObjectMap::create(m_connection.get());
m_connection->setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(true);
m_connection->open(); m_connection->open();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment