Changeset 148286 in webkit


Ignore:
Timestamp:
Apr 12, 2013 10:24:19 AM (11 years ago)
Author:
Carlos Garcia Campos
Message:

[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):

Location:
trunk/Source/WebKit2
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r148284 r148286  
     12013-04-12  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Web Process crash when the UI process finishes too early
     4        https://bugs.webkit.org/show_bug.cgi?id=112729
     5
     6        Reviewed by Anders Carlsson.
     7
     8        The problem is when creating the GSocket in the WorkQeue for the
     9        socket descriptor. GLib considers a programmer error to create a
     10        GSocket providing an invalid socket and finishes the process with
     11        g_error(). We are actually providing a valid socket when creating
     12        the GSocket, but it can be invalidated by the worker thread while
     13        the GSocket is being created. This is because
     14        registerEventSourceHandler is called from the main thread and
     15        unregisterEventSourceHandler can be called from the worker
     16        thread. We are currently registering two event handlers, one to
     17        read data from the socket and another one to close the CoreIPC
     18        connection when the socket connection is broken. Every event
     19        source registered uses its own GSocket (even if the file
     20        descriptor is actually the same), so that when the UI process
     21        finishes too early, the first event handler can be executed in the
     22        worker thread, closing the socket descriptor, while the main
     23        thread is creating the GSocket for the second one.
     24        We don't really need to use a separate event handler to monitor
     25        the connection, because GSocket always notifies when condition is
     26        G_IO_HUP and G_IO_ERR even if they haven't been explicitly set in
     27        g_socket_create_source(). We can register socket event sources
     28        differently, providing also a function to be called when the
     29        connection is closed, using a single socket and the same even source.
     30
     31        * Platform/CoreIPC/unix/ConnectionUnix.cpp:
     32        (CoreIPC::Connection::platformInvalidate):
     33        (CoreIPC::Connection::open): Register a single socket event
     34        handler providing also a function to be called when the connection
     35        is closed.
     36        * Platform/WorkQueue.h:
     37        (WorkQueue):
     38        * Platform/gtk/WorkQueueGtk.cpp: The EventSource class has been
     39        split, moving everyting specific to socket event source to a
     40        derived class SocketEventSource.
     41        (WorkQueue::EventSource::EventSource):
     42        (WorkQueue::EventSource::performWork):
     43        (WorkQueue::EventSource::performWorkOnce):
     44        (WorkQueue::EventSource::performWorkOnTermination):
     45        (WorkQueue::EventSource::deleteEventSource):
     46        (WorkQueue::EventSource):
     47        (WorkQueue::SocketEventSource):
     48        (WorkQueue::SocketEventSource::SocketEventSource):
     49        (WorkQueue::SocketEventSource::cancel):
     50        (WorkQueue::SocketEventSource::didClose):
     51        (WorkQueue::SocketEventSource::checkCondition):
     52        (WorkQueue::SocketEventSource::eventCallback):
     53        (WorkQueue::registerSocketEventHandler):
     54        (WorkQueue::unregisterSocketEventHandler):
     55        (WorkQueue::dispatchOnSource):
     56
    1572013-04-12  Alexey Proskuryakov  <ap@apple.com>
    258

  • trunk/Source/WebKit2/Platform/CoreIPC/unix/ConnectionUnix.cpp

    r141562 r148286  
    141141
    142142#if PLATFORM(GTK)
    143     m_connectionQueue->unregisterEventSourceHandler(m_socketDescriptor);
     143    m_connectionQueue->unregisterSocketEventHandler(m_socketDescriptor);
    144144#endif
    145145
    … …  
    425425    m_socketNotifier = m_connectionQueue->registerSocketEventHandler(m_socketDescriptor, QSocketNotifier::Read, WTF::bind(&Connection::readyReadHandler, this));
    426426#elif PLATFORM(GTK)
    427     m_connectionQueue->registerEventSourceHandler(m_socketDescriptor, (G_IO_HUP | G_IO_ERR), WTF::bind(&Connection::connectionDidClose, this));
    428     m_connectionQueue->registerEventSourceHandler(m_socketDescriptor, G_IO_IN, WTF::bind(&Connection::readyReadHandler, this));
     427    m_connectionQueue->registerSocketEventHandler(m_socketDescriptor, G_IO_IN, WTF::bind(&Connection::readyReadHandler, this), WTF::bind(&Connection::connectionDidClose, this));
    429428#elif PLATFORM(EFL)
    430429    m_connectionQueue->registerSocketEventHandler(m_socketDescriptor, WTF::bind(&Connection::readyReadHandler, this));

  • trunk/Source/WebKit2/Platform/WorkQueue.h

    r142002 r148286  
    8080    void dispatchOnTermination(WebKit::PlatformProcessIdentifier, const Function<void()>&);
    8181#elif PLATFORM(GTK)
    82     void registerEventSourceHandler(int, int, const Function<void()>&);
    83     void unregisterEventSourceHandler(int);
     82    void registerSocketEventHandler(int, int, const Function<void()>& function, const Function<void()>& closeFunction);
     83    void unregisterSocketEventHandler(int);
    8484    void dispatchOnTermination(WebKit::PlatformProcessIdentifier, const Function<void()>&);
    8585#elif PLATFORM(EFL)
    … …  
    164164    Mutex m_eventSourcesLock;
    165165    class EventSource;
    166     HashMap<int, Vector<EventSource*> > m_eventSources;
    167     typedef HashMap<int, Vector<EventSource*> >::iterator EventSourceIterator;
     166    class SocketEventSource;
     167    HashMap<int, Vector<SocketEventSource*> > m_eventSources;
     168    typedef HashMap<int, Vector<SocketEventSource*> >::iterator SocketEventSourceIterator;
    168169#elif PLATFORM(EFL)
    169170    class TimerWorkItem {

  • trunk/Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp

    r147927 r148286  
    3636class WorkQueue::EventSource {
    3737public:
    38     EventSource(const Function<void()>& function, WorkQueue* workQueue, GCancellable* cancellable)
     38    EventSource(const Function<void()>& function, WorkQueue* workQueue)
    3939        : m_function(function)
    4040        , m_workQueue(workQueue)
     41    {
     42        ASSERT(workQueue);
     43    }
     44
     45    void performWork()
     46    {
     47        m_function();
     48    }
     49
     50    static gboolean performWorkOnce(EventSource* eventSource)
     51    {
     52        ASSERT(eventSource);
     53        eventSource->performWork();
     54        return FALSE;
     55    }
     56
     57    static gboolean performWorkOnTermination(GPid, gint, EventSource* eventSource)
     58    {
     59        ASSERT(eventSource);
     60        eventSource->performWork();
     61        return FALSE;
     62    }
     63
     64    static void deleteEventSource(EventSource* eventSource)
     65    {
     66        ASSERT(eventSource);
     67        delete eventSource;
     68    }
     69
     70private:
     71    Function<void()> m_function;
     72    RefPtr<WorkQueue> m_workQueue;
     73};
     74
     75class WorkQueue::SocketEventSource : public WorkQueue::EventSource {
     76public:
     77    SocketEventSource(const Function<void()>& function, WorkQueue* workQueue, int condition, GCancellable* cancellable, const Function<void()>& closeFunction)
     78        : EventSource(function, workQueue)
     79        , m_condition(condition)
    4180        , m_cancellable(cancellable)
    42     {
     81        , m_closeFunction(closeFunction)
     82    {
     83        ASSERT(cancellable);
    4384    }
    4485
    4586    void cancel()
    4687    {
    47         if (!m_cancellable)
    48             return;
    4988        g_cancellable_cancel(m_cancellable);
    5089    }
    5190
    52     static void executeEventSource(EventSource* eventSource)
    53     {
    54         ASSERT(eventSource);
    55         eventSource->m_function();
    56     }
    57 
    58     static gboolean performWorkOnce(EventSource* eventSource)
    59     {
    60         executeEventSource(eventSource);
    61         return FALSE;
    62     }
    63 
    64     static gboolean performWork(GSocket* socket, GIOCondition condition, EventSource* eventSource)
    65     {
    66         if (!(condition & G_IO_IN) && !(condition & G_IO_HUP) && !(condition & G_IO_ERR)) {
    67             // EventSource has been cancelled, return FALSE to destroy the source.
     91    void didClose()
     92    {
     93        m_closeFunction();
     94    }
     95
     96    bool checkCondition(GIOCondition condition) const
     97    {
     98        return condition & m_condition;
     99    }
     100
     101    static gboolean eventCallback(GSocket* socket, GIOCondition condition, SocketEventSource* eventSource)
     102    {
     103        ASSERT(eventSource);
     104
     105        if (condition & G_IO_HUP || condition & G_IO_ERR) {
     106            eventSource->didClose();
    68107            return FALSE;
    69108        }
    70109
    71         executeEventSource(eventSource);
    72         return TRUE;
    73     }
    74 
    75     static gboolean performWorkOnTermination(GPid, gint, EventSource* eventSource)
    76     {
    77         executeEventSource(eventSource);
     110        if (eventSource->checkCondition(condition)) {
     111            eventSource->performWork();
     112            return TRUE;
     113        }
     114
     115        // EventSource has been cancelled, return FALSE to destroy the source.
    78116        return FALSE;
    79117    }
    80118
    81     static void deleteEventSource(EventSource* eventSource)
    82     {
    83         ASSERT(eventSource);
    84         delete eventSource;
    85     }
    86    
    87 public:
    88     Function<void()> m_function;
    89     RefPtr<WorkQueue> m_workQueue;
     119private:
     120    int m_condition;
    90121    GCancellable* m_cancellable;
     122    Function<void()> m_closeFunction;
    91123};
    92124
    … …  
    140172}
    141173
    142 void WorkQueue::registerEventSourceHandler(int fileDescriptor, int condition, const Function<void()>& function)
     174void WorkQueue::registerSocketEventHandler(int fileDescriptor, int condition, const Function<void()>& function, const Function<void()>& closeFunction)
    143175{
    144176    GRefPtr<GSocket> socket = adoptGRef(g_socket_new_from_fd(fileDescriptor, 0));
    … …  
    147179    GRefPtr<GSource> dispatchSource = adoptGRef(g_socket_create_source(socket.get(), static_cast<GIOCondition>(condition), cancellable.get()));
    148180    ASSERT(dispatchSource);
    149     EventSource* eventSource = new EventSource(function, this, cancellable.get());
     181    SocketEventSource* eventSource = new SocketEventSource(function, this, condition, cancellable.get(), closeFunction);
    150182    ASSERT(eventSource);
    151183
    152     g_source_set_callback(dispatchSource.get(), reinterpret_cast<GSourceFunc>(&WorkQueue::EventSource::performWork),
     184    g_source_set_callback(dispatchSource.get(), reinterpret_cast<GSourceFunc>(&WorkQueue::SocketEventSource::eventCallback),
    153185        eventSource, reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
    154186
    … …  
    156188    {
    157189        MutexLocker locker(m_eventSourcesLock);
    158         Vector<EventSource*> sources;
    159         EventSourceIterator it = m_eventSources.find(fileDescriptor);
    160         if (it != m_eventSources.end()) 
     190        Vector<SocketEventSource*> sources;
     191        SocketEventSourceIterator it = m_eventSources.find(fileDescriptor);
     192        if (it != m_eventSources.end())
    161193            sources = it->value;
    162194
    … …  
    168200}
    169201
    170 void WorkQueue::unregisterEventSourceHandler(int fileDescriptor)
     202void WorkQueue::unregisterSocketEventHandler(int fileDescriptor)
    171203{
    172204    ASSERT(fileDescriptor);
    173    
     205
    174206    MutexLocker locker(m_eventSourcesLock);
    175    
    176     EventSourceIterator it = m_eventSources.find(fileDescriptor);
     207
     208    SocketEventSourceIterator it = m_eventSources.find(fileDescriptor);
    177209    ASSERT(it != m_eventSources.end());
    178210    ASSERT(m_eventSources.contains(fileDescriptor));
    179211
    180212    if (it != m_eventSources.end()) {
    181         Vector<EventSource*> sources = it->value;
     213        Vector<SocketEventSource*> sources = it->value;
    182214        for (unsigned i = 0; i < sources.size(); i++)
    183215            sources[i]->cancel();
    … …  
    189221void WorkQueue::dispatchOnSource(GSource* dispatchSource, const Function<void()>& function, GSourceFunc sourceCallback)
    190222{
    191     EventSource* eventSource = new EventSource(function, this, 0);
    192 
    193     g_source_set_callback(dispatchSource, sourceCallback, eventSource,
    194                           reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
     223    g_source_set_callback(dispatchSource, sourceCallback, new EventSource(function, this),
     224        reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
    195225
    196226    g_source_attach(dispatchSource, m_eventContext.get());
Note: See TracChangeset for help on using the changeset viewer.