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
This commit is contained in:
Ingo Weinhold 2006-01-29 01:54:33 +00:00
parent 7baddcf950
commit 1ae42108ec
1 changed files with 23 additions and 7 deletions

View File

@ -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,10 +1172,10 @@ 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?
// 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();
}
}