[Tcl-bugs] [ tcl-Bugs-3574493 ] TclpFinalizeSockets hangs on windows

[SourceForge.net <no...@so...>]

Bugs item #3574493, was opened at 2012-10-04 08:06
Message generated for change (Comment added) made by ferrieux
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3574493&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 27. Channel Types
Group: current: 8.5.12
Status: Open
Resolution: None
Priority: 8
Private: No
Submitted By: Ralf Fassel (ralfixx)
Assigned to: David Gravereaux (davygrvy)
Summary: TclpFinalizeSockets hangs on windows 

Initial Comment:
This seems related to the change introduced by  ID: 1437595.

When running the attached code on Linux, the program opens the server
socket, waits one second and then exits as expected.

When running the same code on Windows-7 against tcl 8.3, the program
behaves as above on Linux.

When running the same code on Windows-7 against tcl 8.5 (Activestate, 
or non-threaded makes no difference) the program blocks
in the _exit() call.  

The reason is to be seen in TclFinalize() where on
Windows it waits for the socket thread to exit when unloading the TCL
DLL, but the corresponding thread either does no longer exist, or does
not get the Message, or does not respond to it:

win/tclWin32Dll.c:
  DLLMain()
        case DLL_PROCESS_DETACH:
        ...
    	Tcl_Finalize();
=>
generic/TclEvent.c
 Tcl_Finalize()
   => Tcl_FinalizeThread()
      => TclFinalizeIOSubsystem();
         => TclpFinalizeSockets();

=>
win/tclWinSock.c
 TclpFinalizeSockets()
 => PostMessage(tsdPtr->hwnd, SOCKET_TERMINATE, 0, 0);
   // this blocks:
    WaitForSingleObject(tsdPtr->readyEvent, INFINITE);

The PostMessage() return code is 0 at that point, indicating an error, 
the corresponding error is 1400/invalid window handle.  
If I change the code to

   if (res) WaitForSingleObject(tsdPtr->readyEvent, INFINITE);

the program terminates as expected.  It seems an error to call
WaitForSingleObject() if the PostMessage() call did not succeed, since
the Wait() relies on the other thread to signal the Wait().  
If the other thread does not get the call, it cannot signal...

Proposed patch against tcl8.5.12:

--- tcl8.5.12/win/tclWinSock.c	2012/09/18 18:43:47	1.1
+++ tcl8.5.12/win/tclWinSock.c	2012/10/04 15:04:50
@@ -461,14 +461,14 @@
     if (tsdPtr != NULL) {
 	if (tsdPtr->socketThread != NULL) {
 	    if (tsdPtr->hwnd != NULL) {
-		PostMessage(tsdPtr->hwnd, SOCKET_TERMINATE, 0, 0);
+		int res = PostMessage(tsdPtr->hwnd, SOCKET_TERMINATE, 0, 0);
 
 		/*
 		 * Wait for the thread to exit. This ensures that we are
 		 * completely cleaned up before we leave this function.
 		 */
 
-		WaitForSingleObject(tsdPtr->readyEvent, INFINITE);
+		if (res) WaitForSingleObject(tsdPtr->readyEvent, INFINITE);
 		tsdPtr->hwnd = NULL;
 	    }
 	    CloseHandle(tsdPtr->socketThread);


----------------------------------------------------------------------

>Comment By: Alexandre Ferrieux (ferrieux)
Date: 2012-10-05 14:50

Message:
Re "it would be nice to know *why*": agreed.
Could you please set breakpoints/printfs at strategic places like
SocketExitHandler , TclpFinalizeSocket, or after the "while" loop in
SocketThread ? I suspect one of the first two is called twice.
Also, what about 8.6 ?

----------------------------------------------------------------------

Comment By: Ralf Fassel (ralfixx)
Date: 2012-10-05 01:14

Message:
Of course the terse form Donal suggested is preferable.  I had introduced
the intermediate var before looking at the exact signature of
PostMessage().
And of course it would be nice to know *why* the handle is already invalid
at that point, but...

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2012-10-04 12:58

Message:
I'd be tempted to write that as:

  if (PostMessage(...)) {
      /* the comment... */
      WaitForSingleObject(...);
  }
  tsdPtr->hwnd = NULL;

YMMV. At least the result of PostMessage() is defined to be boolean...

----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2012-10-04 10:37

Message:
Looks great!

----------------------------------------------------------------------

Comment By: Alexandre Ferrieux (ferrieux)
Date: 2012-10-04 08:36

Message:
Sounds reasonable to me, but my knowledge of Windows (if any) is waning, so
please Dave can you validate ?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3574493&group_id=10894