From 1ae42108ec2d51248ce2182d5dff91550ad19d70 Mon Sep 17 00:00:00 2001
From: Ingo Weinhold <ingo_weinhold@gmx.de>
Date: Sun, 29 Jan 2006 01:54:33 +0000
Subject: [PATCH] Improved the window_quit_loop() situation: * We no longer try
 to dynamic_cast<>() a BLooper* into a BWindow* in   an unsafe context (i.e.
 without the looper or the looper list being   locked). * We no longer try to
 quit windows that haven't been run yet (e.g.   bitmap's offscreen windows).
 Those windows conceptually still belong   to their creator. In the best case
 trying to lock such a window   simply failed (e.g. when the creator was
 another window that had been   told to quit earlier and deleted the window in
 question just not too   early). In worse cases we would dead-lock (when the
 owner has not   been told to quit (or refuses to do so)), cause "locker must
 be   locked" debugger calls or die painfully when accessing an already  
 deleted object.

BTW, I doubt, that the whole window quitting procedure is as it should
be. There's still a huge race condition: When a window is created after
we capture the window list at the beginning of window_quit_loop() that
completely escapes us.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@16134 a95241bf-73f2-0310-859d-f6bbb57e9c96
---
 src/kits/app/Application.cpp | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/kits/app/Application.cpp b/src/kits/app/Application.cpp
index 0e02515d15..5e6a7fd4c0 100644
--- a/src/kits/app/Application.cpp
+++ b/src/kits/app/Application.cpp
@@ -1137,12 +1137,28 @@ BApplication::window_quit_loop(bool quitFilePanels, bool force)
 	BList looperList;
 	{
 		BObjectLocker<BLooperList> listLock(gLooperList);
-		if (listLock.IsLocked())
+		if (listLock.IsLocked()) {
 			gLooperList.GetLooperList(&looperList);
+
+			// Filter the list: We replace the BLooper pointers by BWindow
+			// pointers (dynamic_cast<>() on unlocked loopers is only safe as
+			// long as the looper list is locked!). Furthermore we filter out
+			// windows, that have not been run yet -- those belong to their
+			// creator yet (which also has a lock) and we must not try to
+			// delete them.
+			int32 count = looperList.CountItems();
+			for (int32 i = 0; i < count; i++) {
+				BWindow *window
+					= dynamic_cast<BWindow*>((BLooper*)looperList.ItemAt(i));
+				if (window && window->Thread() < 0)
+					window = NULL;
+				looperList.ReplaceItem(i, window);
+			}
+		}
 	}
 
 	for (int32 i = looperList.CountItems(); i-- > 0; ) {
-		BWindow *window = dynamic_cast<BWindow *>((BLooper *)looperList.ItemAt(i));
+		BWindow *window = (BWindow*)looperList.ItemAt(i);
 
 		// don't quit file panels if we haven't been asked for it
 		if (window == NULL || (!quitFilePanels && window->IsFilePanel()))
@@ -1156,11 +1172,11 @@ BApplication::window_quit_loop(bool quitFilePanels, bool force)
 				return false;
 			}
 
-			// ToDo: QuitRequested() can be overridden by others, IOW we
-			// cannot assume the window hasn't been unlocked in the mean
-			// time and is gone by now.
-			// We probably don't want to die in that case here, do we?
-			window->Quit();
+			// Re-lock, just to make sure that the user hasn't done nasty
+			// things in QuitRequested(). Quit() unlocks fully, thus
+			// double-locking is harmless.
+			if (window->Lock())
+				window->Quit();
 		}
 	}
 	return true;