From 3870c9f18f109903c134d6d36271ec3e83e7a185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Fri, 1 Jul 2005 10:09:11 +0000 Subject: [PATCH] Improved quit mechanism (for the last time): if an application had open windows that wouldn't quit on demand, the app_server would have gotten the kShutdownServer message anyway already (as the last app was quit). Since that one removed things like gDesktop/gBitmapManager, it liked crashing. Now, there is a semaphore that will be send to each app on quit. Only when this semaphore can be acquired, the shutdown message will be sent. Removed unused semaphores (decorator, active app). Replaced fAppListLock with a BLocker (just calling acquire_sem() without error checking is very unsafe in userland, and should never be done). BTW the bug was triggered by broken menu code that only sometimes really quit the window; it leaves a whole lot of zombies around - Stefano, any quick idea? :-) git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@13384 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/servers/app/AppServer.cpp | 59 ++++++++++++++++++----------------- src/servers/app/AppServer.h | 11 ++++--- src/servers/app/ServerApp.cpp | 10 ++++-- src/servers/app/ServerApp.h | 2 +- 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/servers/app/AppServer.cpp b/src/servers/app/AppServer.cpp index bda5b87744..1e3e57b94c 100644 --- a/src/servers/app/AppServer.cpp +++ b/src/servers/app/AppServer.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include "BitmapManager.h" #include "ColorSet.h" @@ -84,8 +85,10 @@ AppServer::AppServer(void) : BApplication (SERVER_SIGNATURE), #else AppServer::AppServer(void) : #endif + fAppListLock("application list"), fCursorSem(-1), - fCursorArea(-1) + fCursorArea(-1), + fShutdownSemaphore(-1) { fMessagePort = create_port(200, SERVER_PORT_NAME); if (fMessagePort == B_NO_MORE_PORTS) @@ -172,12 +175,6 @@ AppServer::AppServer(void) : // Create the bitmap allocator. Object declared in BitmapManager.cpp gBitmapManager = new BitmapManager(); - // This is necessary to mediate access between the Poller and app_server threads - fActiveAppLock = create_sem(1,"app_server_active_sem"); - - // This locker is for app_server and Picasso to vy for control of the ServerApp list - fAppListLock = create_sem(1,"app_server_applist_sem"); - // Spawn our thread-monitoring thread fPicassoThreadID = spawn_thread(PicassoThread, "picasso", B_NORMAL_PRIORITY, this); if (fPicassoThreadID >= 0) @@ -236,7 +233,8 @@ int32 AppServer::PicassoThread(void *data) { while (!sAppServer->fQuitting) { - acquire_sem(sAppServer->fAppListLock); + sAppServer->fAppListLock.Lock(); + for (int32 i = 0;;) { ServerApp *app = (ServerApp *)sAppServer->fAppList.ItemAt(i++); if (!app) @@ -244,7 +242,8 @@ AppServer::PicassoThread(void *data) app->PingTarget(); } - release_sem(sAppServer->fAppListLock); + + sAppServer->fAppListLock.Unlock(); // we do this every second so as not to suck *too* many CPU cycles snooze(1000000); } @@ -444,9 +443,9 @@ AppServer::DispatchMessage(int32 code, BPrivate::PortLink &msg) if (app->InitCheck() == B_OK && app->Run()) { // add the new ServerApp to the known list of ServerApps - acquire_sem(fAppListLock); + fAppListLock.Lock(); fAppList.AddItem(app); - release_sem(fAppListLock); + fAppListLock.Unlock(); } else { delete app; @@ -474,7 +473,7 @@ AppServer::DispatchMessage(int32 code, BPrivate::PortLink &msg) if (msg.Read(&thread) < B_OK) break; - acquire_sem(fAppListLock); + fAppListLock.Lock(); // Run through the list of apps and nuke the proper one @@ -491,13 +490,17 @@ AppServer::DispatchMessage(int32 code, BPrivate::PortLink &msg) } } - release_sem(fAppListLock); + fAppListLock.Unlock(); if (removeApp != NULL) - removeApp->Quit(); + removeApp->Quit(fShutdownSemaphore); + + if (fQuitting && count <= 1) { + // wait for the last app to die + acquire_sem_etc(fShutdownSemaphore, fShutdownCount, B_RELATIVE_TIMEOUT, 500000); - if (fQuitting && count <= 1) PostMessage(kMsgShutdownServer); + } break; } @@ -538,6 +541,11 @@ AppServer::DispatchMessage(int32 code, BPrivate::PortLink &msg) // test apps to quit. This situation will occur only when the server // is compiled as a regular Be application. + fAppListLock.Lock(); + fShutdownSemaphore = create_sem(0, "app_server shutdown"); + fShutdownCount = fAppList.CountItems(); + fAppListLock.Unlock(); + fQuitting = true; BroadcastToAllApps(AS_QUIT_APP); @@ -550,7 +558,7 @@ AppServer::DispatchMessage(int32 code, BPrivate::PortLink &msg) case kMsgShutdownServer: // let's kill all remaining applications - acquire_sem(fAppListLock); + fAppListLock.Lock(); for (int32 i = 0; i < fAppList.CountItems(); i++) { ServerApp *app = (ServerApp*)fAppList.ItemAt(i); @@ -561,7 +569,7 @@ AppServer::DispatchMessage(int32 code, BPrivate::PortLink &msg) kill_thread(fPicassoThreadID); - release_sem(fAppListLock); + fAppListLock.Unlock(); delete gDesktop; delete gBitmapManager; @@ -602,20 +610,15 @@ AppServer::FindApp(const char *signature) if (!signature) return NULL; - acquire_sem(fAppListLock); + BAutolock locker(fAppListLock); - ServerApp *foundApp = NULL; for (int32 i = 0; i < fAppList.CountItems(); i++) { ServerApp *app = (ServerApp*)fAppList.ItemAt(i); - if (app && !strcasecmp(app->Signature(), signature)) { - foundApp = app; - break; - } + if (app && !strcasecmp(app->Signature(), signature)) + return app; } - release_sem(fAppListLock); - - return foundApp; + return NULL; } @@ -631,7 +634,7 @@ AppServer::FindApp(const char *signature) void BroadcastToAllApps(const int32 &code) { - acquire_sem(sAppServer->fAppListLock); + BAutolock locker(sAppServer->fAppListLock); for (int32 i = 0; i < sAppServer->fAppList.CountItems(); i++) { ServerApp *app = (ServerApp *)sAppServer->fAppList.ItemAt(i); @@ -642,8 +645,6 @@ BroadcastToAllApps(const int32 &code) } app->PostMessage(code); } - - release_sem(sAppServer->fAppListLock); } diff --git a/src/servers/app/AppServer.h b/src/servers/app/AppServer.h index 93fb055d64..418ba5d4cd 100644 --- a/src/servers/app/AppServer.h +++ b/src/servers/app/AppServer.h @@ -68,7 +68,9 @@ private: volatile bool fQuitting; + BLocker fAppListLock; BList fAppList; + thread_id fPicassoThreadID; thread_id fISThreadID; @@ -79,11 +81,10 @@ private: port_id fISASPort; port_id fISPort; - - sem_id fActiveAppLock; - sem_id fAppListLock; - sem_id fDecoratorLock; - + + sem_id fShutdownSemaphore; + int32 fShutdownCount; + DisplayDriver *fDriver; }; diff --git a/src/servers/app/ServerApp.cpp b/src/servers/app/ServerApp.cpp index 475e197e16..ab2bec9602 100644 --- a/src/servers/app/ServerApp.cpp +++ b/src/servers/app/ServerApp.cpp @@ -261,7 +261,7 @@ ServerApp::Run() be removed from the application list. */ void -ServerApp::Quit() +ServerApp::Quit(sem_id shutdownSemaphore) { if (fThread < B_OK) { delete this; @@ -273,7 +273,7 @@ ServerApp::Quit() fQuitting = true; PostMessage(kMsgAppQuit); - send_data(fThread, 'QUIT', NULL, 0); + send_data(fThread, 'QUIT', &shutdownSemaphore, sizeof(sem_id)); } @@ -506,9 +506,13 @@ ServerApp::_MessageLooper() // Quit() will send us a message; we're handling the exiting procedure thread_id sender; - receive_data(&sender, NULL, 0); + sem_id shutdownSemaphore; + receive_data(&sender, &shutdownSemaphore, sizeof(sem_id)); delete this; + + if (shutdownSemaphore >= B_OK) + release_sem(shutdownSemaphore); } diff --git a/src/servers/app/ServerApp.h b/src/servers/app/ServerApp.h index c86b67d406..bd7b257a1b 100644 --- a/src/servers/app/ServerApp.h +++ b/src/servers/app/ServerApp.h @@ -44,7 +44,7 @@ public: status_t InitCheck(); bool Run(); - void Quit(); + void Quit(sem_id shutdownSemaphore = -1); /*! \brief Determines whether the application is the active one