From e0574e9130fa19a2e36a68c67c97eefcd57a93d8 Mon Sep 17 00:00:00 2001 From: sletz Date: Wed, 20 Sep 2006 17:36:31 +0000 Subject: [PATCH] On Windows, avoid to use the unsafe Kill thread method. Use thread Stop instead and have blocked IO be unlocked. git-svn-id: http://subversion.jackaudio.org/jack/jack2/trunk/jackmp@1243 0c269be4-1314-0410-8aa9-9f06e86f4224 --- ChangeLog | 7 +- common/JackClient.cpp | 26 +++-- common/JackGraphManager.cpp | 6 +- common/JackLibClient.cpp | 6 +- windows/JackWinEvent.cpp | 3 +- windows/JackWinNamedPipe.cpp | 12 ++- windows/JackWinNamedPipeClientChannel.cpp | 11 ++- windows/JackWinNamedPipeServerChannel.cpp | 115 +++++++++++++--------- windows/JackWinThread.cpp | 7 +- windows/JackdmpWIN32.cpp | 77 +++++++++++++-- windows/jack_metro.dsp | 22 +++++ windows/jackdmp.dsp | 9 ++ windows/libjackdmp.dsp | 6 +- windows/libjackmp.dsp | 6 +- 14 files changed, 235 insertions(+), 78 deletions(-) diff --git a/ChangeLog b/ChangeLog index e781c4d2..31817a0e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,10 @@ Jackdmp changes log --------------------------- +2006-09-20 Stephane Letz + + * On Windows, avoid to use the unsafe Kill thread method. Use thread Stop instead and have blocked IO be unlocked. + 2006-09-16 Stephane Letz * Restore behaviour of LoopBack driver, which has to be opened in any case... @@ -22,6 +26,7 @@ 2006-09-06 Stephane Letz * Correct coreaudio driver (input was not working since 0.55 version). + * Version for 0.58 release. 2006-09-04 Stephane Letz @@ -29,4 +34,4 @@ 2006-09-03 Stephane Letz - * First import of version 0.58 code + * First import of version 0.58 base code diff --git a/common/JackClient.cpp b/common/JackClient.cpp index ec1e0468..716490e7 100644 --- a/common/JackClient.cpp +++ b/common/JackClient.cpp @@ -188,8 +188,8 @@ int JackClient::ClientNotify(int refnum, const char* name, int notify, int sync, res = fXrun(fXrunArg); break; - case JackNotifyChannelInterface::kZombifyClient: - res = fThread->Kill(); + case JackNotifyChannelInterface::kZombifyClient: + //res = fThread->Kill(); Really neede ?? Unsafe in WIN32... JackLog("JackClient::kZombifyClient name = %s ref = %ld \n", name, refnum); ShutDown(); break; @@ -207,10 +207,16 @@ int JackClient::Activate() { JackLog("JackClient::Activate \n"); if (IsActive()) - return 0; - - if (StartThread() < 0) - return -1; + return 0; + + // Done first so that the RT thread then access an allocated synchro + if (!fSynchroTable[GetClientControl()->fRefNum]->Connect(GetClientControl()->fName)) { + jack_error("Cannot ConnectSemaphore %s client", GetClientControl()->fName); + return -1; + } + + if (StartThread() < 0) + return -1; int result = -1; fChannel->ClientActivate(GetClientControl()->fRefNum, &result); @@ -242,7 +248,13 @@ int JackClient::Deactivate() JackLog("JackClient::Deactivate res = %ld \n", result); // We need to wait for the new engine cycle before stopping the RT thread, but this is done by ClientDeactivate - fThread->Kill(); + + // steph + fSynchroTable[GetClientControl()->fRefNum]->Disconnect(); + fThread->Stop(); + + //fThread->Kill(); + return result; } diff --git a/common/JackGraphManager.cpp b/common/JackGraphManager.cpp index 5d8ba5a5..a315c151 100644 --- a/common/JackGraphManager.cpp +++ b/common/JackGraphManager.cpp @@ -747,8 +747,10 @@ const char** JackGraphManager::GetPorts(const char* port_name_pattern, const cha do { cur_index = GetCurrentIndex(); - if (matching_ports) - free(matching_ports); + if (matching_ports) { + free(matching_ports); + JackLog("JackGraphManager::GetPorts retry... \n"); + } matching_ports = GetPortsAux(port_name_pattern, type_name_pattern, flags); next_index = GetCurrentIndex(); } while (cur_index != next_index); // Until a coherent state has been read diff --git a/common/JackLibClient.cpp b/common/JackLibClient.cpp index 840a6b1a..f9e5c087 100644 --- a/common/JackLibClient.cpp +++ b/common/JackLibClient.cpp @@ -99,10 +99,12 @@ int JackLibClient::Open(const char* name) SetupDriverSync(false); // Connect shared synchro : the synchro must be usable in I/O mode when several clients live in the same process - if (!fSynchroTable[fClientControl->fRefNum]->Connect(name)) { + /* + if (!fSynchroTable[fClientControl->fRefNum]->Connect(name)) { jack_error("Cannot ConnectSemaphore %s client", name); goto error; - } + } + */ JackLog("JackLibClient::Open name = %s refnum = %ld\n", name, fClientControl->fRefNum); return 0; diff --git a/windows/JackWinEvent.cpp b/windows/JackWinEvent.cpp index 70126fd3..f87a54c5 100644 --- a/windows/JackWinEvent.cpp +++ b/windows/JackWinEvent.cpp @@ -114,7 +114,8 @@ bool JackWinEvent::ConnectOutput(const char* name) bool JackWinEvent::Disconnect() { if (fEvent) { - JackLog("JackWinEvent::Disconnect %s\n", fName); + JackLog("JackWinEvent::Disconnect %s\n", fName); + SetEvent(fEvent); // to "unlock" threads pending on the event CloseHandle(fEvent); fEvent = NULL; return true; diff --git a/windows/JackWinNamedPipe.cpp b/windows/JackWinNamedPipe.cpp index 0f4d5047..903208aa 100755 --- a/windows/JackWinNamedPipe.cpp +++ b/windows/JackWinNamedPipe.cpp @@ -284,8 +284,10 @@ bool JackWinNamedPipeServer::Accept() JackWinNamedPipeClient* JackWinNamedPipeServer::AcceptClient() { - if (ConnectNamedPipe(fNamedPipe, NULL)) { - return new JackWinNamedPipeClient(fNamedPipe); + if (ConnectNamedPipe(fNamedPipe, NULL)) { + JackWinNamedPipeClient* client = new JackWinNamedPipeClient(fNamedPipe); + // Init the pipe to the default value + fNamedPipe = INVALID_HANDLE_VALUE; } else { switch (GetLastError()) { @@ -302,8 +304,10 @@ JackWinNamedPipeClient* JackWinNamedPipeServer::AcceptClient() int JackWinNamedPipeServer::Close() { - if (fNamedPipe != INVALID_HANDLE_VALUE) { - DisconnectNamedPipe(fNamedPipe); + JackLog("JackWinNamedPipeServer::Close\n"); + + if (fNamedPipe != INVALID_HANDLE_VALUE) { + DisconnectNamedPipe(fNamedPipe); CloseHandle(fNamedPipe); fNamedPipe = INVALID_HANDLE_VALUE; return 0; diff --git a/windows/JackWinNamedPipeClientChannel.cpp b/windows/JackWinNamedPipeClientChannel.cpp index e1001e6e..1b5c1ef2 100644 --- a/windows/JackWinNamedPipeClientChannel.cpp +++ b/windows/JackWinNamedPipeClientChannel.cpp @@ -62,7 +62,9 @@ error: void JackWinNamedPipeClientChannel::Close() { fRequestPipe.Close(); - fNotificationListenPipe.Close(); + fNotificationListenPipe.Close(); + // Here the thread will correctly stop when the pipe are closed + fThread->Stop(); } int JackWinNamedPipeClientChannel::Start() @@ -80,7 +82,7 @@ int JackWinNamedPipeClientChannel::Start() void JackWinNamedPipeClientChannel::Stop() { JackLog("JackWinNamedPipeClientChannel::Stop\n"); - fThread->Kill(); + //fThread->Kill(); Unsafe on WIN32... } void JackWinNamedPipeClientChannel::ServerSyncCall(JackRequest* req, JackResult* res, int* result) @@ -248,8 +250,9 @@ bool JackWinNamedPipeClientChannel::Execute() } return true; -error: - fClient->ShutDown(); +error: + + //fClient->ShutDown(); needed ?? return false; } diff --git a/windows/JackWinNamedPipeServerChannel.cpp b/windows/JackWinNamedPipeServerChannel.cpp index 063e6061..58dcf7c3 100644 --- a/windows/JackWinNamedPipeServerChannel.cpp +++ b/windows/JackWinNamedPipeServerChannel.cpp @@ -43,7 +43,8 @@ JackClientPipeThread::JackClientPipeThread(JackWinNamedPipeClient* pipe) } JackClientPipeThread::~JackClientPipeThread() -{ +{ + JackLog("JackClientPipeThread::~JackClientPipeThread\n"); delete fPipe; delete fThread; } @@ -62,10 +63,18 @@ int JackClientPipeThread::Open(JackServer* server) // Open the Server/Client con } void JackClientPipeThread::Close() // Close the Server/Client connection -{ +{ + JackLog("JackClientPipeThread::Close %x %ld\n", this, fRefNum); + /* + This would hang.. since Close will be followed by a delete, + all ressources will be desallocated at the end. + */ + + /* fThread->Kill(); - fPipe->Close(); - fRefNum = -1; + fPipe->Close(); + */ + fRefNum = -1; } bool JackClientPipeThread::Execute() @@ -111,7 +120,8 @@ int JackClientPipeThread::HandleRequest() if (req.Read(fPipe) == 0) res.fResult = fServer->GetEngine()->ClientClose(req.fRefNum); res.Write(fPipe); - RemoveClient(); + RemoveClient(); + ret = -1; break; } @@ -257,21 +267,20 @@ int JackClientPipeThread::HandleRequest() void JackClientPipeThread::AddClient(char* name, int* shared_engine, int* shared_client, int* shared_ports, int* result) { - JackLog("JackWinNamedPipeServerChannel::AddClient %s\n", name); + JackLog("JackClientPipeThread::AddClient %s\n", name); fRefNum = -1; *result = fServer->GetEngine()->ClientNew(name, &fRefNum, shared_engine, shared_client, shared_ports); } void JackClientPipeThread::RemoveClient() { - JackLog("JackWinNamedPipeServerChannel::RemoveClient ref = %d\n", fRefNum); + JackLog("JackClientPipeThread::RemoveClient ref = %d\n", fRefNum); Close(); } void JackClientPipeThread::KillClient() { - JackLog("JackClientPipeThread::KillClient \n"); - JackLog("JackWinNamedPipeServerChannel::KillClient ref = %d\n", fRefNum); + JackLog("JackClientPipeThread::KillClient ref = %d\n", fRefNum); if (fRefNum == -1) { // Correspond to an already removed client. JackLog("Kill a closed client\n"); @@ -290,7 +299,15 @@ JackWinNamedPipeServerChannel::JackWinNamedPipeServerChannel() } JackWinNamedPipeServerChannel::~JackWinNamedPipeServerChannel() -{ +{ + std::list::iterator it; + + for (it = fClientList.begin(); it != fClientList.end(); it++) { + JackClientPipeThread* client = *it; + client->Close(); + delete client; + } + delete fThread; } @@ -318,12 +335,19 @@ error: fRequestListenPipe.Close(); return -1; } - -void JackWinNamedPipeServerChannel::Close() -{ - fThread->Kill(); - fRequestListenPipe.Close(); -} + +void JackWinNamedPipeServerChannel::Close() +{ + /* + This would hang the server... since we are quitting it, its not really problematic, + all ressources will be desallocated at the end. + */ + + /* + fRequestListenPipe.Close(); + fThread->Stop(); + */ +} bool JackWinNamedPipeServerChannel::Init() { @@ -355,31 +379,34 @@ bool JackWinNamedPipeServerChannel::Execute() } AddClient(pipe); - return true; -} - -void JackWinNamedPipeServerChannel::AddClient(JackWinNamedPipeClient* pipe) -{ - // Remove dead (= not running anymore) clients. - std::list::iterator it = fClientList.begin(); - JackClientPipeThread* client; - while (it != fClientList.end()) { - client = *it; - if (client->IsRunning()) { - it++; - } else { - JackLog("Remove client from list\n"); - it = fClientList.erase(it); - delete(client); - } - } - - client = new JackClientPipeThread(pipe); - client->Open(fServer); - // Here we are sure that the client is running (because it's thread is in "running" state). - fClientList.push_back(client); -} - -} // end of namespace - - + return true; +} + +void JackWinNamedPipeServerChannel::AddClient(JackWinNamedPipeClient* pipe) +{ + // Remove dead (= not running anymore) clients. + std::list::iterator it = fClientList.begin(); + JackClientPipeThread* client; + + JackLog("AddClient size %ld\n", fClientList.size()); + + while (it != fClientList.end()) { + client = *it; + JackLog("Remove dead client = %x running = %ld\n", client, client->IsRunning()); + if (client->IsRunning()) { + it++; + } else { + it = fClientList.erase(it); + delete client; + } + } + + client = new JackClientPipeThread(pipe); + client->Open(fServer); + // Here we are sure that the client is running (because it's thread is in "running" state). + fClientList.push_back(client); +} + +} // end of namespace + + diff --git a/windows/JackWinThread.cpp b/windows/JackWinThread.cpp index b3599470..c1349bfb 100644 --- a/windows/JackWinThread.cpp +++ b/windows/JackWinThread.cpp @@ -167,9 +167,10 @@ int JackWinThread::StartSync() int JackWinThread::Kill() { if (fThread) { // If thread has been started - JackLog("JackWinThread::Kill\n"); - TerminateThread(fThread, 0); /// TO CHECK : dangerous - CloseHandle(fThread); + TerminateThread(fThread, 0); + WaitForSingleObject(fThread, INFINITE); + CloseHandle(fThread); + JackLog("JackWinThread::Kill 2\n"); fThread = NULL; fRunning = false; return 0; diff --git a/windows/JackdmpWIN32.cpp b/windows/JackdmpWIN32.cpp index 55528045..ac5d5f91 100644 --- a/windows/JackdmpWIN32.cpp +++ b/windows/JackdmpWIN32.cpp @@ -204,6 +204,48 @@ static void jack_cleanup_files (const char *server_name) } } } +*/ + +/* +BOOL CtrlHandler( DWORD fdwCtrlType ) +{ + switch( fdwCtrlType ) + { + // Handle the CTRL-C signal. + case CTRL_C_EVENT: + printf( "Ctrl-C event\n\n" ); + Beep( 750, 300 ); + SetEvent(waitEvent); + return( TRUE ); + + // CTRL-CLOSE: confirm that the user wants to exit. + case CTRL_CLOSE_EVENT: + Beep( 600, 200 ); + printf( "Ctrl-Close event\n\n" ); + SetEvent(waitEvent); + return( TRUE ); + + // Pass other signals to the next handler. + case CTRL_BREAK_EVENT: + Beep( 900, 200 ); + printf( "Ctrl-Break event\n\n" ); + return FALSE; + + case CTRL_LOGOFF_EVENT: + Beep( 1000, 200 ); + printf( "Ctrl-Logoff event\n\n" ); + return FALSE; + + case CTRL_SHUTDOWN_EVENT: + Beep( 750, 500 ); + printf( "Ctrl-Shutdown event\n\n" ); + return FALSE; + + default: + return FALSE; + } +} + */ int main(int argc, char* argv[]) @@ -324,19 +366,22 @@ int main(int argc, char* argv[]) if (!seen_driver) { usage (stderr); - exit (1); + //exit (1); + return 0; } drivers = jack_drivers_load (drivers); if (!drivers) { fprintf (stderr, "jackdmp: no drivers found; exiting\n"); - exit (1); + //exit (1); + return 0; } driver_desc = jack_find_driver_descriptor (drivers, driver_name); if (!driver_desc) { fprintf (stderr, "jackdmp: unknown driver '%s'\n", driver_name); - exit (1); + //exit (1); + return 0; } if (optind < argc) { @@ -360,7 +405,8 @@ int main(int argc, char* argv[]) if (jack_parse_driver_params (driver_desc, driver_nargs, driver_args, &driver_params)) { - exit (0); + // exit (0); + return 0; } //if (server_name == NULL) @@ -372,13 +418,16 @@ int main(int argc, char* argv[]) switch (rc) { case EEXIST: fprintf (stderr, "`%s' server already active\n", server_name); - exit (1); + //exit (1); + return 0; case ENOSPC: fprintf (stderr, "too many servers already active\n"); - exit (2); + //exit (2); + return 0; case ENOMEM: fprintf (stderr, "no access to shm registry\n"); - exit (3); + //exit (3); + return 0; default: if (xverbose) fprintf (stderr, "server `%s' registered\n", @@ -401,8 +450,19 @@ int main(int argc, char* argv[]) return 0; } + /* + if( SetConsoleCtrlHandler( (PHANDLER_ROUTINE) CtrlHandler, TRUE ) ) + { + printf( "\nThe Control Handler is installed.\n" ); + } else { + printf( "\nERROR: Could not set control handler"); + } + */ + (void) signal(SIGINT, intrpt); + (void) signal(SIGABRT, intrpt); + (void) signal(SIGTERM, intrpt); if ((res = WaitForSingleObject(waitEvent, INFINITE)) != WAIT_OBJECT_0) { printf("WaitForSingleObject fails err = %ld\n", GetLastError()); @@ -411,7 +471,8 @@ int main(int argc, char* argv[]) /* printf("Type 'q' to quit\n"); while ((c = getchar()) != 'q') {} - */ + */ + JackStop(); diff --git a/windows/jack_metro.dsp b/windows/jack_metro.dsp index 46a3562a..74c10d33 100644 --- a/windows/jack_metro.dsp +++ b/windows/jack_metro.dsp @@ -108,6 +108,28 @@ SOURCE="..\example-clients\metro.c" # Begin Source File SOURCE=.\Release\libjackmp.lib + +!IF "$(CFG)" == "jack_metro - Win32 Release" + +!ELSEIF "$(CFG)" == "jack_metro - Win32 Debug" + +# PROP Exclude_From_Build 1 + +!ENDIF + +# End Source File +# Begin Source File + +SOURCE=.\Debug\libjackmp_debug.lib + +!IF "$(CFG)" == "jack_metro - Win32 Release" + +# PROP Exclude_From_Build 1 + +!ELSEIF "$(CFG)" == "jack_metro - Win32 Debug" + +!ENDIF + # End Source File # End Target # End Project diff --git a/windows/jackdmp.dsp b/windows/jackdmp.dsp index 442fdb9d..40c96cb6 100644 --- a/windows/jackdmp.dsp +++ b/windows/jackdmp.dsp @@ -122,6 +122,15 @@ SOURCE=.\Release\libjackdmp.lib # Begin Source File SOURCE=.\Debug\libjackdmp_debug.lib + +!IF "$(CFG)" == "jackdmp - Win32 Release" + +# PROP Exclude_From_Build 1 + +!ELSEIF "$(CFG)" == "jackdmp - Win32 Debug" + +!ENDIF + # End Source File # End Target # End Project diff --git a/windows/libjackdmp.dsp b/windows/libjackdmp.dsp index ffe862f6..4b755309 100644 --- a/windows/libjackdmp.dsp +++ b/windows/libjackdmp.dsp @@ -42,7 +42,7 @@ RSC=rc.exe # PROP Intermediate_Dir "Release" # PROP Target_Dir "" # ADD BASE CPP /nologo /MT /W3 /GX /O2 /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /D "_MBCS" /D "_USRDLL" /D "LIBJACKDMP_EXPORTS" /YX /FD /c -# ADD CPP /nologo /MD /W3 /GX /O2 /I "." /I "../common" /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /D "_MBCS" /D "_USRDLL" /D "LIBJACKDMP_EXPORTS" /D "__STDC__" /D "REGEX_MALLOC" /D "STDC_HEADERS" /D "__SMP__" /YX /FD /c +# ADD CPP /nologo /MD /W3 /GX /O2 /I "." /I "../common" /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /D "_MBCS" /D "_USRDLL" /D "LIBJACKDMP_EXPORTS" /D "__STDC__" /D "REGEX_MALLOC" /D "STDC_HEADERS" /D "__SMP__" /FR /YX /FD /c # ADD BASE MTL /nologo /D "NDEBUG" /mktyplib203 /win32 # ADD MTL /nologo /D "NDEBUG" /mktyplib203 /win32 # ADD BASE RSC /l 0x40c /d "NDEBUG" @@ -241,6 +241,10 @@ SOURCE=.\resource.rc SOURCE=..\common\shm.c # End Source File +# Begin Source File + +SOURCE="..\..\..\..\pthreads-win32\pthreadVC2.lib" +# End Source File # End Group # Begin Group "Header Files" diff --git a/windows/libjackmp.dsp b/windows/libjackmp.dsp index 283f7ff6..3733cae1 100644 --- a/windows/libjackmp.dsp +++ b/windows/libjackmp.dsp @@ -42,7 +42,7 @@ RSC=rc.exe # PROP Intermediate_Dir "Release" # PROP Target_Dir "" # ADD BASE CPP /nologo /MT /W3 /GX /O2 /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /D "_MBCS" /D "_USRDLL" /D "LIBJACKMP_EXPORTS" /YX /FD /c -# ADD CPP /nologo /MD /W3 /GX /O2 /I "." /I "../common" /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /D "_MBCS" /D "_USRDLL" /D "LIBJACKMP_EXPORTS" /D "__STDC__" /D "REGEX_MALLOC" /D "STDC_HEADERS" /D "__SMP__" /YX /FD /c +# ADD CPP /nologo /MD /W3 /GX /O2 /I "." /I "../common" /I "../../../../pthreads-win32" /D "WIN32" /D "NDEBUG" /D "_WINDOWS" /D "_MBCS" /D "_USRDLL" /D "LIBJACKMP_EXPORTS" /D "__STDC__" /D "REGEX_MALLOC" /D "STDC_HEADERS" /D "__SMP__" /YX /FD /c # ADD BASE MTL /nologo /D "NDEBUG" /mktyplib203 /win32 # ADD MTL /nologo /D "NDEBUG" /mktyplib203 /win32 # ADD BASE RSC /l 0x40c /d "NDEBUG" @@ -186,5 +186,9 @@ SOURCE=..\common\shm.c # PROP Default_Filter "ico;cur;bmp;dlg;rc2;rct;bin;rgs;gif;jpg;jpeg;jpe" # End Group +# Begin Source File + +SOURCE="..\..\..\..\pthreads-win32\pthreadVC2.lib" +# End Source File # End Target # End Project