Modified: trunk/Source/WebKit2/ChangeLog (148285 => 148286)
--- trunk/Source/WebKit2/ChangeLog 2013-04-12 17:23:42 UTC (rev 148285)
+++ trunk/Source/WebKit2/ChangeLog 2013-04-12 17:24:19 UTC (rev 148286)
@@ -1,3 +1,59 @@
+2013-04-12 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [GTK] Web Process crash when the UI process finishes too early
+ https://bugs.webkit.org/show_bug.cgi?id=112729
+
+ Reviewed by Anders Carlsson.
+
+ The problem is when creating the GSocket in the WorkQeue for the
+ socket descriptor. GLib considers a programmer error to create a
+ GSocket providing an invalid socket and finishes the process with
+ g_error(). We are actually providing a valid socket when creating
+ the GSocket, but it can be invalidated by the worker thread while
+ the GSocket is being created. This is because
+ registerEventSourceHandler is called from the main thread and
+ unregisterEventSourceHandler can be called from the worker
+ thread. We are currently registering two event handlers, one to
+ read data from the socket and another one to close the CoreIPC
+ connection when the socket connection is broken. Every event
+ source registered uses its own GSocket (even if the file
+ descriptor is actually the same), so that when the UI process
+ finishes too early, the first event handler can be executed in the
+ worker thread, closing the socket descriptor, while the main
+ thread is creating the GSocket for the second one.
+ We don't really need to use a separate event handler to monitor
+ the connection, because GSocket always notifies when condition is
+ G_IO_HUP and G_IO_ERR even if they haven't been explicitly set in
+ g_socket_create_source(). We can register socket event sources
+ differently, providing also a function to be called when the
+ connection is closed, using a single socket and the same even source.
+
+ * Platform/CoreIPC/unix/ConnectionUnix.cpp:
+ (CoreIPC::Connection::platformInvalidate):
+ (CoreIPC::Connection::open): Register a single socket event
+ handler providing also a function to be called when the connection
+ is closed.
+ * Platform/WorkQueue.h:
+ (WorkQueue):
+ * Platform/gtk/WorkQueueGtk.cpp: The EventSource class has been
+ split, moving everyting specific to socket event source to a
+ derived class SocketEventSource.
+ (WorkQueue::EventSource::EventSource):
+ (WorkQueue::EventSource::performWork):
+ (WorkQueue::EventSource::performWorkOnce):
+ (WorkQueue::EventSource::performWorkOnTermination):
+ (WorkQueue::EventSource::deleteEventSource):
+ (WorkQueue::EventSource):
+ (WorkQueue::SocketEventSource):
+ (WorkQueue::SocketEventSource::SocketEventSource):
+ (WorkQueue::SocketEventSource::cancel):
+ (WorkQueue::SocketEventSource::didClose):
+ (WorkQueue::SocketEventSource::checkCondition):
+ (WorkQueue::SocketEventSource::eventCallback):
+ (WorkQueue::registerSocketEventHandler):
+ (WorkQueue::unregisterSocketEventHandler):
+ (WorkQueue::dispatchOnSource):
+
2013-04-12 Alexey Proskuryakov <a...@apple.com>
<rdar://problem/13126204> [Mac] Tweak sandbox profile.
Modified: trunk/Source/WebKit2/Platform/CoreIPC/unix/ConnectionUnix.cpp (148285 => 148286)
--- trunk/Source/WebKit2/Platform/CoreIPC/unix/ConnectionUnix.cpp 2013-04-12 17:23:42 UTC (rev 148285)
+++ trunk/Source/WebKit2/Platform/CoreIPC/unix/ConnectionUnix.cpp 2013-04-12 17:24:19 UTC (rev 148286)
@@ -140,7 +140,7 @@
return;
#if PLATFORM(GTK)
- m_connectionQueue->unregisterEventSourceHandler(m_socketDescriptor);
+ m_connectionQueue->unregisterSocketEventHandler(m_socketDescriptor);
#endif
#if PLATFORM(QT)
@@ -424,8 +424,7 @@
#if PLATFORM(QT)
m_socketNotifier = m_connectionQueue->registerSocketEventHandler(m_socketDescriptor, QSocketNotifier::Read, WTF::bind(&Connection::readyReadHandler, this));
#elif PLATFORM(GTK)
- m_connectionQueue->registerEventSourceHandler(m_socketDescriptor, (G_IO_HUP | G_IO_ERR), WTF::bind(&Connection::connectionDidClose, this));
- m_connectionQueue->registerEventSourceHandler(m_socketDescriptor, G_IO_IN, WTF::bind(&Connection::readyReadHandler, this));
+ m_connectionQueue->registerSocketEventHandler(m_socketDescriptor, G_IO_IN, WTF::bind(&Connection::readyReadHandler, this), WTF::bind(&Connection::connectionDidClose, this));
#elif PLATFORM(EFL)
m_connectionQueue->registerSocketEventHandler(m_socketDescriptor, WTF::bind(&Connection::readyReadHandler, this));
#endif
Modified: trunk/Source/WebKit2/Platform/WorkQueue.h (148285 => 148286)
--- trunk/Source/WebKit2/Platform/WorkQueue.h 2013-04-12 17:23:42 UTC (rev 148285)
+++ trunk/Source/WebKit2/Platform/WorkQueue.h 2013-04-12 17:24:19 UTC (rev 148286)
@@ -79,8 +79,8 @@
QSocketNotifier* registerSocketEventHandler(int, QSocketNotifier::Type, const Function<void()>&);
void dispatchOnTermination(WebKit::PlatformProcessIdentifier, const Function<void()>&);
#elif PLATFORM(GTK)
- void registerEventSourceHandler(int, int, const Function<void()>&);
- void unregisterEventSourceHandler(int);
+ void registerSocketEventHandler(int, int, const Function<void()>& function, const Function<void()>& closeFunction);
+ void unregisterSocketEventHandler(int);
void dispatchOnTermination(WebKit::PlatformProcessIdentifier, const Function<void()>&);
#elif PLATFORM(EFL)
void registerSocketEventHandler(int, const Function<void()>&);
@@ -163,8 +163,9 @@
GRefPtr<GMainLoop> m_eventLoop;
Mutex m_eventSourcesLock;
class EventSource;
- HashMap<int, Vector<EventSource*> > m_eventSources;
- typedef HashMap<int, Vector<EventSource*> >::iterator EventSourceIterator;
+ class SocketEventSource;
+ HashMap<int, Vector<SocketEventSource*> > m_eventSources;
+ typedef HashMap<int, Vector<SocketEventSource*> >::iterator SocketEventSourceIterator;
#elif PLATFORM(EFL)
class TimerWorkItem {
public:
Modified: trunk/Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp (148285 => 148286)
--- trunk/Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp 2013-04-12 17:23:42 UTC (rev 148285)
+++ trunk/Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp 2013-04-12 17:24:19 UTC (rev 148286)
@@ -35,59 +35,91 @@
// WorkQueue::EventSource
class WorkQueue::EventSource {
public:
- EventSource(const Function<void()>& function, WorkQueue* workQueue, GCancellable* cancellable)
+ EventSource(const Function<void()>& function, WorkQueue* workQueue)
: m_function(function)
, m_workQueue(workQueue)
- , m_cancellable(cancellable)
{
+ ASSERT(workQueue);
}
- void cancel()
+ void performWork()
{
- if (!m_cancellable)
- return;
- g_cancellable_cancel(m_cancellable);
+ m_function();
}
- static void executeEventSource(EventSource* eventSource)
+ static gboolean performWorkOnce(EventSource* eventSource)
{
ASSERT(eventSource);
- eventSource->m_function();
+ eventSource->performWork();
+ return FALSE;
}
- static gboolean performWorkOnce(EventSource* eventSource)
+ static gboolean performWorkOnTermination(GPid, gint, EventSource* eventSource)
{
- executeEventSource(eventSource);
+ ASSERT(eventSource);
+ eventSource->performWork();
return FALSE;
}
- static gboolean performWork(GSocket* socket, GIOCondition condition, EventSource* eventSource)
+ static void deleteEventSource(EventSource* eventSource)
{
- if (!(condition & G_IO_IN) && !(condition & G_IO_HUP) && !(condition & G_IO_ERR)) {
- // EventSource has been cancelled, return FALSE to destroy the source.
- return FALSE;
- }
+ ASSERT(eventSource);
+ delete eventSource;
+ }
- executeEventSource(eventSource);
- return TRUE;
+private:
+ Function<void()> m_function;
+ RefPtr<WorkQueue> m_workQueue;
+};
+
+class WorkQueue::SocketEventSource : public WorkQueue::EventSource {
+public:
+ SocketEventSource(const Function<void()>& function, WorkQueue* workQueue, int condition, GCancellable* cancellable, const Function<void()>& closeFunction)
+ : EventSource(function, workQueue)
+ , m_condition(condition)
+ , m_cancellable(cancellable)
+ , m_closeFunction(closeFunction)
+ {
+ ASSERT(cancellable);
}
- static gboolean performWorkOnTermination(GPid, gint, EventSource* eventSource)
+ void cancel()
{
- executeEventSource(eventSource);
- return FALSE;
+ g_cancellable_cancel(m_cancellable);
}
- static void deleteEventSource(EventSource* eventSource)
+ void didClose()
{
+ m_closeFunction();
+ }
+
+ bool checkCondition(GIOCondition condition) const
+ {
+ return condition & m_condition;
+ }
+
+ static gboolean eventCallback(GSocket* socket, GIOCondition condition, SocketEventSource* eventSource)
+ {
ASSERT(eventSource);
- delete eventSource;
+
+ if (condition & G_IO_HUP || condition & G_IO_ERR) {
+ eventSource->didClose();
+ return FALSE;
+ }
+
+ if (eventSource->checkCondition(condition)) {
+ eventSource->performWork();
+ return TRUE;
+ }
+
+ // EventSource has been cancelled, return FALSE to destroy the source.
+ return FALSE;
}
-
-public:
- Function<void()> m_function;
- RefPtr<WorkQueue> m_workQueue;
+
+private:
+ int m_condition;
GCancellable* m_cancellable;
+ Function<void()> m_closeFunction;
};
// WorkQueue
@@ -139,25 +171,25 @@
g_main_loop_run(m_eventLoop.get());
}
-void WorkQueue::registerEventSourceHandler(int fileDescriptor, int condition, const Function<void()>& function)
+void WorkQueue::registerSocketEventHandler(int fileDescriptor, int condition, const Function<void()>& function, const Function<void()>& closeFunction)
{
GRefPtr<GSocket> socket = adoptGRef(g_socket_new_from_fd(fileDescriptor, 0));
ASSERT(socket);
GRefPtr<GCancellable> cancellable = adoptGRef(g_cancellable_new());
GRefPtr<GSource> dispatchSource = adoptGRef(g_socket_create_source(socket.get(), static_cast<GIOCondition>(condition), cancellable.get()));
ASSERT(dispatchSource);
- EventSource* eventSource = new EventSource(function, this, cancellable.get());
+ SocketEventSource* eventSource = new SocketEventSource(function, this, condition, cancellable.get(), closeFunction);
ASSERT(eventSource);
- g_source_set_callback(dispatchSource.get(), reinterpret_cast<GSourceFunc>(&WorkQueue::EventSource::performWork),
+ g_source_set_callback(dispatchSource.get(), reinterpret_cast<GSourceFunc>(&WorkQueue::SocketEventSource::eventCallback),
eventSource, reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
// Set up the event sources under the mutex since this is shared across multiple threads.
{
MutexLocker locker(m_eventSourcesLock);
- Vector<EventSource*> sources;
- EventSourceIterator it = m_eventSources.find(fileDescriptor);
- if (it != m_eventSources.end())
+ Vector<SocketEventSource*> sources;
+ SocketEventSourceIterator it = m_eventSources.find(fileDescriptor);
+ if (it != m_eventSources.end())
sources = it->value;
sources.append(eventSource);
@@ -167,18 +199,18 @@
g_source_attach(dispatchSource.get(), m_eventContext.get());
}
-void WorkQueue::unregisterEventSourceHandler(int fileDescriptor)
+void WorkQueue::unregisterSocketEventHandler(int fileDescriptor)
{
ASSERT(fileDescriptor);
-
+
MutexLocker locker(m_eventSourcesLock);
-
- EventSourceIterator it = m_eventSources.find(fileDescriptor);
+
+ SocketEventSourceIterator it = m_eventSources.find(fileDescriptor);
ASSERT(it != m_eventSources.end());
ASSERT(m_eventSources.contains(fileDescriptor));
if (it != m_eventSources.end()) {
- Vector<EventSource*> sources = it->value;
+ Vector<SocketEventSource*> sources = it->value;
for (unsigned i = 0; i < sources.size(); i++)
sources[i]->cancel();
@@ -188,11 +220,9 @@
void WorkQueue::dispatchOnSource(GSource* dispatchSource, const Function<void()>& function, GSourceFunc sourceCallback)
{
- EventSource* eventSource = new EventSource(function, this, 0);
+ g_source_set_callback(dispatchSource, sourceCallback, new EventSource(function, this),
+ reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
- g_source_set_callback(dispatchSource, sourceCallback, eventSource,
- reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
-
g_source_attach(dispatchSource, m_eventContext.get());
}
