Changeset 74967 in webkit


Ignore:
Timestamp:
Jan 4, 2011 6:01:47 AM (13 years ago)
Author:
kbalazs@webkit.org
Message:

2011-01-04 Balazs Kelemen <​kbalazs@webkit.org>

Reviewed by Kenneth Rohde Christiansen.

[Qt][WK2] Incomplete clean up on termination
​https://bugs.webkit.org/show_bug.cgi?id=51474

Do not kill the web process to force it's termination.
Fix object destruction and cleanup logic and make the cleanup more reliable.
Do not try to cleanup on crash because it is not reliable.

  • Platform/CoreIPC/qt/ConnectionQt.cpp: (CoreIPC::Connection::platformInvalidate): Do not delete the socket if we do not own it. This is the case with a server connection.
  • Platform/qt/MappedMemoryPool.cpp: Turn MappedMemoryPool into a QObject so the CleanupHandler can destruct it in the same way as the other objects. (WebKit::MappedMemoryPool::instance): (WebKit::MappedMemoryPool::~MappedMemoryPool):
  • Platform/qt/MappedMemoryPool.h: (WebKit::MappedMemoryPool::MappedMemoryPool):
  • Platform/qt/SharedMemoryQt.cpp: (WebKit::SharedMemory::create): No need to care about QCoreApplication::aboutToQuit. It is handled by the CleanupHandler. (WebKit::SharedMemory::~SharedMemory): Ditto.
  • Shared/qt/CleanupHandler.cpp: Renamed from WebKit2/Shared/qt/CrashHandler.cpp. No longer try to handle crash but just normal termination. (WebKit::CleanupHandler::CleanupHandler): Connect deleteObjects with QCoreApplication::aboutToQuit. Ensure appropriate thread affinity. (WebKit::CleanupHandler::sigTermHandler): Stop the main event loop. (WebKit::CleanupHandler::deleteObjects): Use deleteLater instead of delete since it is more reliable.
  • Shared/qt/CleanupHandler.h: Renamed from WebKit2/Shared/qt/CrashHandler.h. (WebKit::CleanupHandler::instance): (WebKit::CleanupHandler::markForCleanup): (WebKit::CleanupHandler::unmark):
  • UIProcess/Launcher/qt/ProcessLauncherQt.cpp: Do not kill the web process immidiately but give it a chance to cleanup. Use QLocalServer::removeServer for assuring that the socket file will be removed. (WebKit::ProcessLauncherHelper::serverName): (WebKit::cleanupAtExit): Renamed from cleanupProcesses. Only kill the web process when it times out terminating. Fixed the bug of changing the list while iterating it over by disconnecting from the processStateChanged slot. (WebKit::QtWebProcess::QtWebProcess): Added missing meta type registration of QProcess::ProcessState. (WebKit::ProcessLauncherHelper::~ProcessLauncherHelper): (WebKit::ProcessLauncherHelper::ProcessLauncherHelper): No need to add the the instance to the CleanupHandler because we will remove the socket file in cleanupAtExit. (WebKit::ProcessLauncherHelper::instance): (WebKit::ProcessLauncher::terminateProcess):
  • WebKit2.pro:
Location:
trunk/WebKit2
Files:
7 edited
2 moved

Legend:

Unmodified
Added
Removed

  • trunk/WebKit2/ChangeLog

    r74964 r74967  
     12011-01-04  Balazs Kelemen  <kbalazs@webkit.org>
     2
     3        Reviewed by Kenneth Rohde Christiansen.
     4
     5        [Qt][WK2] Incomplete clean up on termination
     6        https://bugs.webkit.org/show_bug.cgi?id=51474
     7
     8        Do not kill the web process to force it's termination.
     9        Fix object destruction and cleanup logic and make the cleanup more reliable.
     10        Do not try to cleanup on crash because it is not reliable.
     11
     12        * Platform/CoreIPC/qt/ConnectionQt.cpp:
     13        (CoreIPC::Connection::platformInvalidate): Do not delete the socket if we
     14        do not own it. This is the case with a server connection.
     15        * Platform/qt/MappedMemoryPool.cpp:
     16        Turn MappedMemoryPool into a QObject so the CleanupHandler can destruct it
     17        in the same way as the other objects.
     18        (WebKit::MappedMemoryPool::instance):
     19        (WebKit::MappedMemoryPool::~MappedMemoryPool):
     20        * Platform/qt/MappedMemoryPool.h:
     21        (WebKit::MappedMemoryPool::MappedMemoryPool):
     22        * Platform/qt/SharedMemoryQt.cpp:
     23        (WebKit::SharedMemory::create): No need to care about QCoreApplication::aboutToQuit.
     24        It is handled by the CleanupHandler.
     25        (WebKit::SharedMemory::~SharedMemory): Ditto.
     26        * Shared/qt/CleanupHandler.cpp: Renamed from WebKit2/Shared/qt/CrashHandler.cpp.
     27        No longer try to handle crash but just normal termination.
     28        (WebKit::CleanupHandler::CleanupHandler): Connect deleteObjects with QCoreApplication::aboutToQuit.
     29        Ensure appropriate thread affinity.
     30        (WebKit::CleanupHandler::sigTermHandler): Stop the main event loop.
     31        (WebKit::CleanupHandler::deleteObjects): Use deleteLater instead of delete since it is more reliable.
     32        * Shared/qt/CleanupHandler.h: Renamed from WebKit2/Shared/qt/CrashHandler.h.
     33        (WebKit::CleanupHandler::instance):
     34        (WebKit::CleanupHandler::markForCleanup):
     35        (WebKit::CleanupHandler::unmark):
     36        * UIProcess/Launcher/qt/ProcessLauncherQt.cpp:
     37        Do not kill the web process immidiately but give it a chance to cleanup.
     38        Use QLocalServer::removeServer for assuring that the socket file will be removed.
     39        (WebKit::ProcessLauncherHelper::serverName):
     40        (WebKit::cleanupAtExit): Renamed from cleanupProcesses. Only kill the web process when it times
     41        out terminating. Fixed the bug of changing the list while iterating it over by disconnecting
     42        from the processStateChanged slot.
     43        (WebKit::QtWebProcess::QtWebProcess): Added missing meta type registration of QProcess::ProcessState.
     44        (WebKit::ProcessLauncherHelper::~ProcessLauncherHelper):
     45        (WebKit::ProcessLauncherHelper::ProcessLauncherHelper): No need to add the the instance to the
     46        CleanupHandler because we will remove the socket file in cleanupAtExit.
     47        (WebKit::ProcessLauncherHelper::instance):
     48        (WebKit::ProcessLauncher::terminateProcess):
     49        * WebKit2.pro:
     50
    1512011-01-04  Benjamin Poulain  <benjamin.poulain@nokia.com>
    252

  • trunk/WebKit2/Platform/CoreIPC/qt/ConnectionQt.cpp

    r74355 r74967  
    5151void Connection::platformInvalidate()
    5252{
    53     delete m_socket;
     53    m_socket->disconnect();
     54    if (!m_isServer)
     55        m_socket->deleteLater();
    5456    m_socket = 0;
    5557}

  • trunk/WebKit2/Platform/qt/MappedMemoryPool.cpp

    r71919 r74967  
    2828#include "MappedMemoryPool.h"
    2929
     30#include "CleanupHandler.h"
    3031#include "StdLibExtras.h"
    3132#include <QDir>
    … …  
    3536namespace WebKit {
    3637
     38MappedMemoryPool* MappedMemoryPool::theInstance = 0;
     39
     40MappedMemoryPool* MappedMemoryPool::instance()
     41{
     42    if (!theInstance) {
     43        theInstance = new MappedMemoryPool;
     44
     45        // Do not leave mapping files on the disk.
     46        CleanupHandler::instance()->markForCleanup(theInstance);
     47    }
     48
     49    return theInstance;
     50}
     51
    3752MappedMemoryPool::~MappedMemoryPool()
    3853{
    39     clear();
    40 }
     54    CleanupHandler::instance()->unmark(theInstance);
    4155
    42 void MappedMemoryPool::clear()
    43 {
    4456    for (unsigned n = 0; n < m_pool.size(); ++n) {
    4557        MappedMemory& current = m_pool.at(n);

  • trunk/WebKit2/Platform/qt/MappedMemoryPool.h

    r71729 r74967  
    3030
    3131#include <QFile>
     32#include <QObject>
    3233#include <wtf/StdLibExtras.h>
    3334#include <wtf/Vector.h>
    … …  
    8788};
    8889
    89 class MappedMemoryPool {
     90class MappedMemoryPool : QObject {
     91    Q_OBJECT
    9092public:
    91     static MappedMemoryPool* instance()
    92     {
    93         DEFINE_STATIC_LOCAL(MappedMemoryPool, singleton, ());
    94         return &singleton;
    95     }
     93    static MappedMemoryPool* instance();
    9694
    9795    MappedMemory* mapMemory(size_t size);
    9896    MappedMemory* mapFile(QString fileName, size_t size);
    9997
    100     void clear();
     98private:
     99    MappedMemoryPool() { }
     100    ~MappedMemoryPool();
    101101
    102 private:
    103     MappedMemoryPool() { };
    104     ~MappedMemoryPool();
     102    static MappedMemoryPool* theInstance;
    105103
    106104    Vector<MappedMemory> m_pool;

  • trunk/WebKit2/Platform/qt/SharedMemoryQt.cpp

    r71638 r74967  
    3030#include "ArgumentDecoder.h"
    3131#include "ArgumentEncoder.h"
    32 #include "CrashHandler.h"
     32#include "CleanupHandler.h"
    3333#include "WebCoreArgumentCoders.h"
    3434#include <unistd.h>
    … …  
    100100    sharedMemory->m_data = impl->data();
    101101
    102     impl->connect(QCoreApplication::instance(), SIGNAL(aboutToQuit()), SLOT(deleteLater()));
    103 
    104     // Release the shared memory segment even on crash!
    105     CrashHandler::instance()->markForDeletionOnCrash(impl);
     102    // Do not leave the shared memory segment behind.
     103    CleanupHandler::instance()->markForCleanup(impl);
    106104
    107105    return sharedMemory.release();
    … …  
    139137    sharedMemory->m_data = impl->data();
    140138
    141     impl->connect(QCoreApplication::instance(), SIGNAL(aboutToQuit()), SLOT(deleteLater()));
    142 
    143     // Release the shared memory segment even on crash!
    144     CrashHandler::instance()->markForDeletionOnCrash(impl);
     139    // Do not leave the shared memory segment behind.
     140    CleanupHandler::instance()->markForCleanup(impl);
    145141
    146142    return sharedMemory.release();
    … …  
    149145SharedMemory::~SharedMemory()
    150146{
    151     // Avoid double deletion when deleteLater has already been called through the signal-slot connection.
    152     if (QCoreApplication::instance()->disconnect(m_impl)) {
    153         // m_impl must be non-null and it must point to a valid QSharedMemory object.
    154         ASSERT(qobject_cast<QSharedMemory*>(m_impl));
    155         delete m_impl;
    156     }
    157 
    158     CrashHandler::instance()->didDelete(m_impl);
     147    CleanupHandler::instance()->unmark(m_impl);
     148    delete m_impl;
    159149}
    160150

  • trunk/WebKit2/Shared/qt/CleanupHandler.cpp

    r74966 r74967  
    2424 */
    2525
    26 #include "CrashHandler.h"
     26#include "CleanupHandler.h"
    2727
    2828#include "MappedMemoryPool.h"
     29#include "RunLoop.h"
    2930#include <csignal>
    3031#include <cstdlib>
    31 #include <wtf/AlwaysInline.h>
     32#include <QApplication>
    3233
    3334namespace WebKit {
    3435
    35 CrashHandler* CrashHandler::theInstance = 0;
     36CleanupHandler* CleanupHandler::theInstance = 0;
    3637
    37 CrashHandler::CrashHandler()
     38CleanupHandler::CleanupHandler()
    3839    : m_inDeleteObjects(false)
    3940{
    40     signal(SIGABRT, &CrashHandler::signalHandler);
    41     signal(SIGBUS, &CrashHandler::signalHandler);
    42     signal(SIGILL, &CrashHandler::signalHandler);
    43     signal(SIGINT, &CrashHandler::signalHandler);
    44     signal(SIGFPE, &CrashHandler::signalHandler);
    45     signal(SIGQUIT, &CrashHandler::signalHandler);
    46     signal(SIGSEGV, &CrashHandler::signalHandler);
    47     signal(SIGTRAP, &CrashHandler::signalHandler);
     41    moveToThread(qApp->thread()); // Ensure that we are acting on the main thread.
     42    connect(qApp, SIGNAL(aboutToQuit()), SLOT(deleteObjects()), Qt::DirectConnection);
     43    signal(SIGTERM, &CleanupHandler::sigTermHandler);
    4844}
    4945
    50 NO_RETURN void CrashHandler::signalHandler(int)
     46void CleanupHandler::sigTermHandler(int)
    5147{
    52     CrashHandler::theInstance->deleteObjects();
    53     exit(EXIT_FAILURE);
     48    ::RunLoop::main()->stop();
    5449}
    5550
    56 void CrashHandler::deleteObjects()
     51void CleanupHandler::deleteObjects()
    5752{
    5853    m_inDeleteObjects = true;
    59     qDeleteAll(m_objects);
    60 
    61     MappedMemoryPool::instance()->clear();
     54    for (unsigned i = 0; i < m_objects.size(); ++i)
     55        m_objects[i]->deleteLater();
    6256}
    6357

  • trunk/WebKit2/Shared/qt/CleanupHandler.h

    r74966 r74967  
    2424 */
    2525
    26 #ifndef CrashHandler_h
    27 #define CrashHandler_h
     26#ifndef CleanupHandler_h
     27#define CleanupHandler_h
    2828
     29#include <QCoreApplication>
    2930#include <QList>
    3031#include <QObject>
    … …  
    3435namespace WebKit {
    3536
    36 class CrashHandler : private QObject {
     37class CleanupHandler : private QObject {
    3738    Q_OBJECT
    3839public:
    39     static CrashHandler* instance()
     40    static CleanupHandler* instance()
    4041    {
    4142        if (!theInstance)
    42             theInstance = new CrashHandler();
     43            theInstance = new CleanupHandler();
    4344        return theInstance;
    4445    }
    4546
    46     void markForDeletionOnCrash(QObject* object)
     47    void markForCleanup(QObject* object)
    4748    {
    48         theInstance->m_objects.append(object);
     49        m_objects.append(object);
    4950    }
    5051
    51     void didDelete(QObject* object)
     52    void unmark(QObject* object)
    5253    {
    5354        if (m_inDeleteObjects)
    5455            return;
    55         theInstance->m_objects.removeOne(object);
     56        m_objects.removeOne(object);
    5657    }
    5758
     59private slots:
     60    void deleteObjects();
     61
    5862private:
    59     static void signalHandler(int);
    60     static CrashHandler* theInstance;
     63    static void sigTermHandler(int);
     64    static CleanupHandler* theInstance;
    6165
    62     CrashHandler();
    63 
    64     void deleteObjects();
     66    CleanupHandler();
    6567
    6668    QList<QObject*> m_objects;
    … …  
    7072} // namespace WebKit
    7173
    72 #endif // CrashHandler_h
     74#endif // CleanupHandler_h

  • trunk/WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp

    r74108 r74967  
    2828
    2929#include "Connection.h"
     30#include "CleanupHandler.h"
    3031#include "NotImplemented.h"
    3132#include "RunLoop.h"
    32 #include "CrashHandler.h"
    3333#include "WebProcess.h"
    3434#include <runtime/InitializeThreading.h>
    … …  
    4343#include <QFile>
    4444#include <QLocalServer>
     45#include <QMetaType>
    4546#include <QProcess>
     47#include <QString>
    4648
    4749#include <QtCore/qglobal.h>
    … …  
    6163    QLocalSocket* takePendingConnection();
    6264    static ProcessLauncherHelper* instance();
     65
     66    const QString serverName() const { return m_server.serverName(); }
     67
    6368private:
    6469    ProcessLauncherHelper();
    … …  
    7176Q_GLOBAL_STATIC(WTF::HashSet<QProcess*>, processes);
    7277
    73 static void cleanupProcesses()
    74 {
    75     WTF::HashSet<QProcess*>::const_iterator it = processes()->begin();
    76     while (it != processes()->end()) {
    77         (*it)->kill();
    78         ++it;
    79     }
     78static void cleanupAtExit()
     79{
     80    // Terminate our web process(es).
     81    WTF::HashSet<QProcess*>::const_iterator end = processes()->end();
     82    for (WTF::HashSet<QProcess*>::const_iterator it = processes()->begin(); it != end; ++it) {
     83        QProcess* process = *it;
     84        process->disconnect(process);
     85        process->terminate();
     86        if (!process->waitForFinished(200))
     87            process->kill();
     88    }
     89
     90    // Do not leave the socket file behind.
     91    QLocalServer::removeServer(ProcessLauncherHelper::instance()->serverName());
    8092}
    8193
    … …  
    8799        : QProcess(parent)
    88100    {
     101        static bool isRegistered = false;
     102        if (!isRegistered) {
     103            qRegisterMetaType<QProcess::ProcessState>("QProcess::ProcessState");
     104            isRegistered = true;
     105        }
     106
    89107        connect(this, SIGNAL(stateChanged(QProcess::ProcessState)), this, SLOT(processStateChanged(QProcess::ProcessState)));
    90108    }
    … …  
    142160{
    143161    m_server.close();
    144     CrashHandler::instance()->didDelete(this);
    145162}
    146163
    … …  
    153170    }
    154171    connect(&m_server, SIGNAL(newConnection()), this, SLOT(newConnection()));
    155     connect(QCoreApplication::instance(), SIGNAL(aboutToQuit()), SLOT(deleteLater()), Qt::QueuedConnection);
    156 
    157     // Do not leave socket files on the disk even on crash!
    158     CrashHandler::instance()->markForDeletionOnCrash(this);
    159172}
    160173
    … …  
    168181        processes()->clear();
    169182
    170         atexit(cleanupProcesses);
     183        atexit(cleanupAtExit);
    171184    }
    172185    return result;
    … …  
    193206
    194207    QObject::connect(m_processIdentifier, SIGNAL(finished(int)), m_processIdentifier, SLOT(deleteLater()), Qt::QueuedConnection);
    195     m_processIdentifier->kill();
     208    m_processIdentifier->terminate();
    196209}
    197210

  • trunk/WebKit2/WebKit2.pro

    r74862 r74967  
    278278    Shared/WebUserContentURLPattern.h \
    279279    Shared/Plugins/Netscape/NetscapePluginModule.h \
    280     Shared/qt/CrashHandler.h \
     280    Shared/qt/CleanupHandler.h \
    281281    Shared/qt/PlatformCertificateInfo.h \
    282282    Shared/qt/UpdateChunk.h \
    … …  
    488488    Shared/WebWheelEvent.cpp \
    489489    Shared/qt/BackingStoreQt.cpp \
    490     Shared/qt/CrashHandler.cpp \
     490    Shared/qt/CleanupHandler.cpp \
    491491    Shared/qt/NativeWebKeyboardEventQt.cpp \
    492492    Shared/qt/UpdateChunk.cpp \
Note: See TracChangeset for help on using the changeset viewer.