From dd84f111b9cac2b7f1bae6802784a166a2e22dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Thu, 18 Oct 2007 13:49:36 +0000 Subject: [PATCH] Fixed a few potential deadlocks: * in r22410 the unlock/relock was removed accidently from ServerWindow::_Hide() before calling a desktop method. * Desktop::ActivateWindow() no longer calls SetWorkspace() with the window lock held. * WorkspacesLayer::MouseUp() now uses the asynchronous version of SetWorkspace(). * AFAICT AS_HIDE_WINDOW, AS_SHOW_WINDOW, and AS_MINIMIZE_WINDOW don't need the all window lock, reverted to standard single window lock. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22614 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/servers/app/Desktop.cpp | 44 ++++++++++++++--------------- src/servers/app/ServerWindow.cpp | 20 ++++--------- src/servers/app/WorkspacesLayer.cpp | 2 +- 3 files changed, 29 insertions(+), 37 deletions(-) diff --git a/src/servers/app/Desktop.cpp b/src/servers/app/Desktop.cpp index 0b26437c30..e50e3657ae 100644 --- a/src/servers/app/Desktop.cpp +++ b/src/servers/app/Desktop.cpp @@ -618,7 +618,8 @@ Desktop::_DispatchMessage(int32 code, BPrivate::LinkReceiver &link) PostMessage(kMsgQuitLooper); break; - case AS_ACTIVATE_WORKSPACE: { + case AS_ACTIVATE_WORKSPACE: + { int32 index; link.Read(&index); @@ -814,7 +815,6 @@ Desktop::SetWorkspacesCount(int32 newCount) /*! Changes the current workspace to the one specified by \a index. - You must not hold any window lock when calling this method. */ void Desktop::SetWorkspaceAsync(int32 index) @@ -836,7 +836,8 @@ Desktop::SetWorkspace(int32 index) LockAllWindows(); DesktopSettings settings(this); - if (index < 0 || index >= settings.WorkspacesCount() || index == fCurrentWorkspace) { + if (index < 0 || index >= settings.WorkspacesCount() + || index == fCurrentWorkspace) { UnlockAllWindows(); return; } @@ -1390,29 +1391,15 @@ Desktop::ActivateWindow(WindowLayer* window) fFront = NULL; return; } - - bool windowOnOtherWorkspace = !window->InWorkspace(fCurrentWorkspace); - - if (windowOnOtherWorkspace - && (window->Flags() & B_NO_WORKSPACE_ACTIVATION) - && !(window->Flags() & B_NOT_ANCHORED_ON_ACTIVATE)) + if (window->Workspaces() == 0) return; // TODO: take care about floating windows - if (!LockAllWindows()) - return; - + bool windowOnOtherWorkspace = !window->InWorkspace(fCurrentWorkspace); if (windowOnOtherWorkspace) { - // if the window wants to come to the current workspace, - // do so here - else activate the workspace on which this - // window is - if (window->Flags() & B_NOT_ANCHORED_ON_ACTIVATE) { - // bring the window to the current workspace - // TODO: what if this window is on multiple workspaces?!? - uint32 workspaces = workspace_to_workspaces(fCurrentWorkspace); - SetWindowWorkspaces(window, workspaces); - } else { + if ((window->Flags() & B_NO_WORKSPACE_ACTIVATION) == 0 + && (window->Flags() & B_NOT_ANCHORED_ON_ACTIVATE) == 0) { // switch to the workspace on which this window is // (we'll take the first one that the window is on) uint32 workspaces = window->Workspaces(); @@ -1420,10 +1407,23 @@ Desktop::ActivateWindow(WindowLayer* window) uint32 workspace = workspace_to_workspaces(i); if (workspaces & workspace) { SetWorkspace(i); + windowOnOtherWorkspace = false; break; } } - } + } else if ((window->Flags() & B_NOT_ANCHORED_ON_ACTIVATE) == 0) + return; + } + + if (!LockAllWindows()) + return; + + if (windowOnOtherWorkspace + && (window->Flags() & B_NOT_ANCHORED_ON_ACTIVATE) != 0) { + // bring the window to the current workspace + // TODO: what if this window is on multiple workspaces?!? + uint32 workspaces = workspace_to_workspaces(fCurrentWorkspace); + SetWindowWorkspaces(window, workspaces); } if (window == FrontWindow()) { diff --git a/src/servers/app/ServerWindow.cpp b/src/servers/app/ServerWindow.cpp index de5ba3cf8d..52a99d5d05 100644 --- a/src/servers/app/ServerWindow.cpp +++ b/src/servers/app/ServerWindow.cpp @@ -368,16 +368,11 @@ ServerWindow::_Show() if (fQuitting || !fWindowLayer->IsHidden() || fWindowLayer->IsOffscreenWindow()) return; -// TODO: deadlock. Desktop::ShowWindow() will eventually lock the event thread, -// which might be blocking on the all window lock with it's own lock already -// head. -// Maybe we need to dispatch a message to the desktop to show/hide us -// instead of doing it from this thread. -//fDesktop->UnlockSingleWindow(); -fDesktop->UnlockAllWindows(); + // TODO: Maybe we need to dispatch a message to the desktop to show/hide us + // instead of doing it from this thread. + fDesktop->UnlockSingleWindow(); fDesktop->ShowWindow(fWindowLayer); -fDesktop->LockAllWindows(); -//fDesktop->LockSingleWindow(); + fDesktop->LockSingleWindow(); if (fDirectWindowData != NULL) HandleDirectConnection(B_DIRECT_START | B_BUFFER_RESET); @@ -397,9 +392,9 @@ ServerWindow::_Hide() if (fDirectWindowData != NULL) HandleDirectConnection(B_DIRECT_STOP); -// TODO: race condition? maybe we need to dispatch a message to the desktop to show/hide us -// instead of doing it from this thread. + fDesktop->UnlockSingleWindow(); fDesktop->HideWindow(fWindowLayer); + fDesktop->LockSingleWindow(); } @@ -3195,9 +3190,6 @@ bool ServerWindow::_MessageNeedsAllWindowsLocked(uint32 code) const { switch (code) { - case AS_SHOW_WINDOW: - case AS_HIDE_WINDOW: - case AS_MINIMIZE_WINDOW: case AS_ACTIVATE_WINDOW: case AS_SET_WINDOW_TITLE: case AS_ADD_TO_SUBSET: diff --git a/src/servers/app/WorkspacesLayer.cpp b/src/servers/app/WorkspacesLayer.cpp index 981af577df..85a8fdc79b 100644 --- a/src/servers/app/WorkspacesLayer.cpp +++ b/src/servers/app/WorkspacesLayer.cpp @@ -432,7 +432,7 @@ WorkspacesLayer::MouseUp(BMessage* message, BPoint where) int32 index; _WorkspaceAt(where, index); if (index >= 0) - Window()->Desktop()->SetWorkspace(index); + Window()->Desktop()->SetWorkspaceAsync(index); } if (fSelectedWindow != NULL) {