On a crash we left over socket files from the beginning of our wk2 port. Since the usage of QSharedMemory (http://trac.webkit.org/changeset/71118) the situation is even worst. We do not delete those even on normal termination because their owner (SharedMemory) is not deleted. This is because of the policy in WebKit to avoid destructors on exit. There is at least one shared memory segment that we left over, namely the one that is used by VisitedLinkTable. Unix system does not clean up those segments after the process dies or terminates. (Yesterday we were needed to shutdown our wk2 bot because the amount of left over shared memory semaphores has reached the kernel limit). I would like to find a solution to clean up those things on terminate and on crash as well.
Test case (on Linux): 1. launch MiniBrowser and close it 2. ipcs - in the output you can see the left over segments and semaphores: ------ Shared Memory Segments -------- key shmid owner perms bytes nattch status <other process's staff> 0x510038a6 822312969 balazs 666 4096 0 <- from VisitedLinkTable ------ Semaphore Arrays -------- key semid owner perms nsems 0x510038a5 268795904 balazs 666 1 To clear them, use these commands: ipcrm -s <semid> ipcrm -m <shmid>
Created attachment 72928 [details] proposed patch I am using signal.h on every platform. I do not really know what are the differences on Windows and Symbian but I tried a tiny example on Windows with MinGW and it worked. After a quick googling I found that it should work on Symbian as well. I think the definition of HAVE_SIGNAL_H in Platform.h is wrong.
Attachment 72928 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit2/ChangeLog', u'WebKit2/Platform/qt/SharedMemoryQt.cpp', u'WebKit2/Shared/qt/CrashHandler.cpp', u'WebKit2/Shared/qt/CrashHandler.h', u'WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp', u'WebKit2/WebKit2.pro']" exit_code: 1 WebKit2/Shared/qt/CrashHandler.cpp:53: Missing space before ( in foreach( [whitespace/parens] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 72928 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=72928&action=review Looks good in general, some problems though: > WebKit2/Shared/qt/CrashHandler.cpp:38 > +CrashHandler::CrashHandler() > +{ Missing initialization of m_inDeleteObjects! > WebKit2/Shared/qt/CrashHandler.cpp:41 > + signal(SIGABRT, &CrashHandler::signalHandler); > + signal(SIGSEGV, &CrashHandler::signalHandler); > + signal(SIGINT, &CrashHandler::signalHandler); You probably want to catch SIGFPE, SIGILL, SIGQUIT, SIGTRAP and SIGBUS as well here. > WebKit2/Shared/qt/CrashHandler.cpp:54 > + foreach(QObject* object, m_objects) > + delete object; qDeleteAll(m_objects); > WebKit2/Shared/qt/CrashHandler.h:48 > + void addToDeletion(QObject* object) addToDeletion() sounds weird to me. deleteOnCrash() perhaps? markForDeletionOnCrash()?
Created attachment 72940 [details] Patch Changed regarding to Kling's comments
Comment on attachment 72940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72940&action=review r=me, three small things: > WebKit2/Shared/qt/CrashHandler.cpp:30 > +#include <QCoreApplication> Is this include needed? > WebKit2/Shared/qt/CrashHandler.h:34 > +#include <QCoreApplication> Is this include needed? > WebKit2/Shared/qt/CrashHandler.h:74 > +#endif // CleanupHandler_h #endif // CrashHandler_h > WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp:119 > + // Do not leave socket files on the disk even on crash! > + CrashHandler::instance()->markForDeletionOnCrash(this); This call is missing a corresponding didDelete() in ~ProcessLauncherHelper()
(In reply to comment #6) > (From update of attachment 72940 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=72940&action=review > > r=me, three small things: > > > WebKit2/Shared/qt/CrashHandler.cpp:30 > > +#include <QCoreApplication> > > Is this include needed? No > > > WebKit2/Shared/qt/CrashHandler.h:34 > > +#include <QCoreApplication> > > Is this include needed? No :) > > > WebKit2/Shared/qt/CrashHandler.h:74 > > +#endif // CleanupHandler_h > > #endif // CrashHandler_h > > > WebKit2/UIProcess/Launcher/qt/ProcessLauncherQt.cpp:119 > > + // Do not leave socket files on the disk even on crash! > > + CrashHandler::instance()->markForDeletionOnCrash(this); > > This call is missing a corresponding didDelete() in ~ProcessLauncherHelper() I omit that because ProcessLauncherHelper lives as long as the process, however we can CRASH even after ~ProcessLauncherHelper() is called so you are right. I will fix the issues (and do final a smoke test) before landing.
Committed in http://trac.webkit.org/changeset/71409
Comment on attachment 72940 [details] Patch Clearing flags