From dbc03773ef3b00c0daf4f4cdd8871f0a4072d094 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Tue, 28 Aug 2007 21:19:15 +0000 Subject: [PATCH] Added a SIGCHLD handler, so that the terminal will notice when one of its shells has been terminated. Usually the thread reading from the tty master would notice when all slaves have been closed, but they won't be closed when the shell started a background job that's still living. Unfortunately there are race conditions in the terminal that can lead to deadlock when a session is closed. The service threads usually happily lock the window, while the (locked) window would wait for the service threads to quit. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22100 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/apps/terminal/Shell.h | 3 +- src/apps/terminal/TermApp.cpp | 67 +++++++++++++++++++++++++++---- src/apps/terminal/TermApp.h | 4 ++ src/apps/terminal/TermConst.h | 1 + src/apps/terminal/TermView.cpp | 16 ++++++++ src/apps/terminal/TermView.h | 2 + src/apps/terminal/TermWindow.cpp | 68 ++++++++++++++++++++++++++++---- src/apps/terminal/TermWindow.h | 2 + 8 files changed, 146 insertions(+), 17 deletions(-) diff --git a/src/apps/terminal/Shell.h b/src/apps/terminal/Shell.h index fb48fe59ce..d8037983b5 100644 --- a/src/apps/terminal/Shell.h +++ b/src/apps/terminal/Shell.h @@ -37,7 +37,8 @@ public: status_t GetAttr(struct termios &attr); status_t SetAttr(struct termios &attr); - int FD() const; + int FD() const; + pid_t ProcessID() const { return fProcessID; } virtual void ViewAttached(TermView *view); virtual void ViewDetached(); diff --git a/src/apps/terminal/TermApp.cpp b/src/apps/terminal/TermApp.cpp index 7f1e6ac56b..0e962f465d 100644 --- a/src/apps/terminal/TermApp.cpp +++ b/src/apps/terminal/TermApp.cpp @@ -9,11 +9,11 @@ #include "TermApp.h" -#include "Arguments.h" -#include "CodeConv.h" -#include "PrefHandler.h" -#include "TermWindow.h" -#include "TermConst.h" +#include +#include +#include +#include +#include #include #include @@ -24,9 +24,11 @@ #include #include -#include -#include -#include +#include "Arguments.h" +#include "CodeConv.h" +#include "PrefHandler.h" +#include "TermWindow.h" +#include "TermConst.h" static bool sUsageRequested = false; @@ -74,6 +76,18 @@ TermApp::ReadyToRun() if (sUsageRequested) return; + // Install a SIGCHLD signal handler, so that we will be notified, when + // a shell exits. + struct sigaction action; + action.sa_handler = (sighandler_t)_SigChildHandler; + sigemptyset(&action.sa_mask); + action.sa_flags = SA_NODEFER; + action.sa_userdata = this; + if (sigaction(SIGCHLD, &action, NULL) < 0) { + fprintf(stderr, "sigaction() failed: %s\n", strerror(errno)); + // continue anyway + } + status_t status = _MakeTermWindow(fTermFrame); // failed spawn, print stdout and open alert panel @@ -143,6 +157,10 @@ TermApp::MessageReceived(BMessage* msg) break; } + case MSG_CHECK_CHILDREN: + _HandleChildCleanup(); + break; + default: BApplication::MessageReceived(msg); break; @@ -473,6 +491,39 @@ TermApp::_RegisterTerminal() //} +void +TermApp::_HandleChildCleanup() +{ +} + + +/*static*/ void +TermApp::_SigChildHandler(int signal, void* data) +{ + // Spawing a thread that does the actual signal handling is pretty much + // the only safe thing to do in a multi-threaded application. The + // interrupted thread might have been anywhere, e.g. in a critical section, + // holding locks. If we do anything that does require locking at any point + // (e.g. memory allocation, messaging), we risk a dead-lock or data + // structure corruption. Spawing a thread is safe though, since its only + // a system call. + thread_id thread = spawn_thread(_ChildCleanupThread, "child cleanup", + B_NORMAL_PRIORITY, ((TermApp*)data)->fTermWindow); + if (thread >= 0) + resume_thread(thread); +} + + +/*static*/ status_t +TermApp::_ChildCleanupThread(void* data) +{ + // Just drop the windowa message and let it do the actual work. This + // saves us additional synchronization measures. + return ((TermWindow*)data)->PostMessage(MSG_CHECK_CHILDREN); +} + + + void TermApp::_Usage(char *name) { diff --git a/src/apps/terminal/TermApp.h b/src/apps/terminal/TermApp.h index 619a4089a5..52a2f565f2 100644 --- a/src/apps/terminal/TermApp.h +++ b/src/apps/terminal/TermApp.h @@ -63,6 +63,10 @@ class TermApp : public BApplication { void _UnregisterTerminal(); void _RegisterTerminal(); + void _HandleChildCleanup(); + static void _SigChildHandler(int signal, void* data); + static status_t _ChildCleanupThread(void* data); + void _Usage(char *name); bool fStartFullscreen; diff --git a/src/apps/terminal/TermConst.h b/src/apps/terminal/TermConst.h index e47e544903..b225f97bdf 100644 --- a/src/apps/terminal/TermConst.h +++ b/src/apps/terminal/TermConst.h @@ -83,6 +83,7 @@ const uint32 FULLSCREEN = 'fscr'; const uint32 MSG_FONT_CHANGED = 'fntc'; const uint32 SAVE_AS_DEFAULT = 'sadf'; +const uint32 MSG_CHECK_CHILDREN = 'ckch'; // Preference Read/Write Keys const char* const PREF_HALF_FONT_FAMILY = "Half Font Famly"; diff --git a/src/apps/terminal/TermView.cpp b/src/apps/terminal/TermView.cpp index 3cae54468b..92f4facde5 100644 --- a/src/apps/terminal/TermView.cpp +++ b/src/apps/terminal/TermView.cpp @@ -2521,6 +2521,22 @@ TermView::NotifyQuit(int32 reason) } +void +TermView::CheckShellGone() +{ + if (!fShell) + return; + + // check, if the shell does still live + pid_t pid = fShell->ProcessID(); + team_info info; + if (get_team_info(pid, &info) == B_BAD_TEAM_ID) { + // the shell is gone + NotifyQuit(0); + } +} + + inline void TermView::_Redraw(int x1, int y1, int x2, int y2) { diff --git a/src/apps/terminal/TermView.h b/src/apps/terminal/TermView.h index f402581976..a32b7a97a3 100644 --- a/src/apps/terminal/TermView.h +++ b/src/apps/terminal/TermView.h @@ -115,6 +115,8 @@ public: bool Find(const BString &str, bool forwardSearch, bool matchCase, bool matchWord); void GetSelection(BString &str); + void CheckShellGone(); + protected: virtual void AttachedToWindow(); virtual void DetachedFromWindow(); diff --git a/src/apps/terminal/TermWindow.cpp b/src/apps/terminal/TermWindow.cpp index 8595f0d706..b6b030620a 100644 --- a/src/apps/terminal/TermWindow.cpp +++ b/src/apps/terminal/TermWindow.cpp @@ -254,12 +254,15 @@ TermWindow::MessageReceived(BMessage *message) break; case kCloseView: - // TODO: We assume that this message was sent from the current active tab. - // Since the implementation of BTabView uses AddChild/RemoveChild on the - // views, the current active tab is the only one who is attached, thus - // the only one which could send a message. Change this. - _RemoveTab(fTabView->Selection()); - break; + { + TermView* termView; + if (message->FindPointer("termView", (void**)&termView) == B_OK) { + int32 index = _IndexOfTermView(termView); + if (index >= 0) + _RemoveTab(index); + } + break; + } case MENU_NEW_TERM: { @@ -455,12 +458,12 @@ TermWindow::MessageReceived(BMessage *message) case MSG_FONT_CHANGED: PostMessage(MSG_HALF_FONT_CHANGED); break; - + case MSG_COLOR_CHANGED: _SetTermColors(_ActiveTermView()); _ActiveTermView()->Invalidate(); break; - + case SAVE_AS_DEFAULT: { BPath path; @@ -476,6 +479,10 @@ TermWindow::MessageReceived(BMessage *message) _DoPrint(); break; + case MSG_CHECK_CHILDREN: + _CheckChildren(); + break; + case B_ABOUT_REQUESTED: be_app->PostMessage(B_ABOUT_REQUESTED); break; @@ -659,6 +666,50 @@ TermWindow::_ActiveTermView() } +int32 +TermWindow::_IndexOfTermView(TermView* termView) const +{ + if (!termView) + return -1; + + // find the view + int32 count = fTabView->CountTabs(); + for (int32 i = count - 1; i >= 0; i--) { + BScrollView* scrollView + = dynamic_cast(fTabView->ViewForTab(i)); + if (!scrollView) + continue; + + if (termView == scrollView->Target()) + return i; + } + + return -1; +} + + +void +TermWindow::_CheckChildren() +{ + // There seems to be no separate list of sessions, so we have to iterate + // through the tabs. + int32 count = fTabView->CountTabs(); + for (int32 i = count - 1; i >= 0; i--) { + // get the term view + BScrollView* scrollView + = dynamic_cast(fTabView->ViewForTab(i)); + if (!scrollView) + continue; + TermView* termView = dynamic_cast(scrollView->Target()); + if (!termView) + continue; + + termView->CheckShellGone(); + } +} + + + // CustomTermView CustomTermView::CustomTermView(int32 rows, int32 columns, int32 argc, const char **argv, int32 historySize) : @@ -672,6 +723,7 @@ CustomTermView::NotifyQuit(int32 reason) { if (Window()) { BMessage message(kCloseView); + message.AddPointer("termView", this); message.AddInt32("reason", reason); Window()->PostMessage(&message); } diff --git a/src/apps/terminal/TermWindow.h b/src/apps/terminal/TermWindow.h index a28396d3ec..7b7c648022 100644 --- a/src/apps/terminal/TermWindow.h +++ b/src/apps/terminal/TermWindow.h @@ -67,6 +67,8 @@ private: void _AddTab(Arguments *args); void _RemoveTab(int32 index); TermView* _ActiveTermView(); + int32 _IndexOfTermView(TermView* termView) const; + void _CheckChildren(); SmartTabView *fTabView; TermView *fTermView;