Changeset 150808 in webkit


Ignore:
Timestamp:
May 28, 2013 10:12:07 AM (11 years ago)
Author:
lauro.neto@openbossa.org
Message:

[GTK] Connection issues in repeated WebProcess crash/reloads.
​https://bugs.webkit.org/show_bug.cgi?id=115880

Reviewed by Anders Carlsson.

When stressing the WebProcess creation/destruction, WebKitGTK can
often run into socket issues like bad file descriptor errors or
polling a socket indefinitely.

Currently WebKitGTK has three places where a socket can be
closed.

  • childFinishedFunction (in ProcessLauncherGtk.cpp)
  • Connection::platformInvalidate (in ConnectionUnix.cpp)
  • WorkQueue EventSource destruction (in WorkQueueGtk.cpp)

To avoid these race conditions, socket closing will be handled
by the event source callback in WorkQueueGtk.cpp.

  • Platform/CoreIPC/unix/ConnectionUnix.cpp:

(CoreIPC::Connection::platformInvalidate): Do not close the socket
when the connection is invalidated, the socket event source is
unregistered in this method and the socket is closed when the
GSocket associated to the event source is destroyed.

  • UIProcess/Launcher/gtk/ProcessLauncherGtk.cpp:

(WebKit::ProcessLauncher::launchProcess): Do not monitor child
process to close the connection on termination. This was needed in
the past when we used DGRAM sockets, we currently use always
connection oriented sockets, so that when the other end closes
the connection we are notified and the connection is invalidated.

Location:
trunk/Source/WebKit2
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r150806 r150808  
     12013-05-28  Lauro Neto <lauro.neto@openbossa.org>
     2
     3        [GTK] Connection issues in repeated WebProcess crash/reloads.
     4        https://bugs.webkit.org/show_bug.cgi?id=115880
     5
     6        Reviewed by Anders Carlsson.
     7
     8        When stressing the WebProcess creation/destruction, WebKitGTK can
     9        often run into socket issues like bad file descriptor errors or
     10        polling a socket indefinitely.
     11
     12        Currently WebKitGTK has three places where a socket can be
     13        closed.
     14
     15        - childFinishedFunction (in ProcessLauncherGtk.cpp)
     16        - Connection::platformInvalidate (in ConnectionUnix.cpp)
     17        - WorkQueue EventSource destruction (in WorkQueueGtk.cpp)
     18
     19        To avoid these race conditions, socket closing will be handled
     20        by the event source callback in WorkQueueGtk.cpp.
     21
     22        * Platform/CoreIPC/unix/ConnectionUnix.cpp:
     23        (CoreIPC::Connection::platformInvalidate): Do not close the socket
     24        when the connection is invalidated, the socket event source is
     25        unregistered in this method and the socket is closed when the
     26        GSocket associated to the event source is destroyed.
     27        * UIProcess/Launcher/gtk/ProcessLauncherGtk.cpp:
     28        (WebKit::ProcessLauncher::launchProcess): Do not monitor child
     29        process to close the connection on termination. This was needed in
     30        the past when we used DGRAM sockets, we currently use always
     31        connection oriented sockets, so that when the other end closes
     32        the connection we are notified and the connection is invalidated.
     33
    1342013-05-28  Martin Robinson  <mrobinson@igalia.com>
    235

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

    r148286 r150808  
    134134void Connection::platformInvalidate()
    135135{
     136    // In GTK+ platform the socket is closed by the work queue.
     137#if !PLATFORM(GTK)
    136138    if (m_socketDescriptor != -1)
    137139        while (close(m_socketDescriptor) == -1 && errno == EINTR) { }
     140#endif
    138141
    139142    if (!m_isConnected)

  • trunk/Source/WebKit2/UIProcess/Launcher/gtk/ProcessLauncherGtk.cpp

    r149192 r150808  
    6363}
    6464
    65 static void childFinishedFunction(GPid, gint status, gpointer userData)
    66 {
    67     if (WIFEXITED(status) && !WEXITSTATUS(status))
    68         return;
    69 
    70     close(GPOINTER_TO_INT(userData));
    71 }
    72 
    7365void ProcessLauncher::launchProcess()
    7466{
    … …  
    10193
    10294    GOwnPtr<GError> error;
    103     int spawnFlags = G_SPAWN_LEAVE_DESCRIPTORS_OPEN | G_SPAWN_DO_NOT_REAP_CHILD;
    104     if (!g_spawn_async(0, argv, 0, static_cast<GSpawnFlags>(spawnFlags), childSetupFunction, GINT_TO_POINTER(sockets[1]), &pid, &error.outPtr())) {
     95    if (!g_spawn_async(0, argv, 0, G_SPAWN_LEAVE_DESCRIPTORS_OPEN, childSetupFunction, GINT_TO_POINTER(sockets[1]), &pid, &error.outPtr())) {
    10596        g_printerr("Unable to fork a new WebProcess: %s.\n", error->message);
    10697        ASSERT_NOT_REACHED();
    … …  
    109100    close(sockets[0]);
    110101    m_processIdentifier = pid;
    111 
    112     // Monitor the child process, it calls waitpid to prevent the child process from becomming a zombie,
    113     // and it allows us to close the socket when the child process crashes.
    114     g_child_watch_add(m_processIdentifier, childFinishedFunction, GINT_TO_POINTER(sockets[1]));
    115102
    116103    // We've finished launching the process, message back to the main run loop.
Note: See TracChangeset for help on using the changeset viewer.