I think the only changes I'd make is to not make the variables in
SysSemaphore public and define methods for the different operations
that might change/test the state.
Rick
On Fri, Feb 5, 2010 at 1:14 PM, Mark Miesfeld <miesf...@gmail.com> wrote:
> On Fri, Feb 5, 2010 at 2:00 AM, Rick McGuire <object.r...@gmail.com> wrote:
>
>> This actually looks better, but I suspect that the hang is now
>> occurring because the reentrant windows proc is creating a condition
>> where this thread is never getting a chance to be woken up. I didn't
>> really expect this chance to make the oodialog problem go way, but
>> rather to keep the internal interpreter state from getting corrupted.
>
> Rick,
>
> Yes, the call stacks I looked at did not look as corrupted as they did before.
>
> Below is the thread local storage change that fixes the ooDialog
> problem for you to review. (Well along with the change to
> relinquish() in Windows that I committed last night.)
>
> * It is confined to Windows dependent files.
>
> * It reuses the old RexxSetProcessMessages() API.
>
> It would have been cleaner, I think, to put the global vaiables needed
> in SystemInterpreter. But, since SysSemaphore is linked into
> rxapi.dll and rexxapi.exe, the convultions needed to get it to compile
> were just too much.
>
> I've tested it yesterday and today, including running the test suite
> with and don't see any problems.
>
> Index: interpreter/platform/windows/MiscSystem.cpp
> ===================================================================
> --- interpreter/platform/windows/MiscSystem.cpp (revision 5539)
> +++ interpreter/platform/windows/MiscSystem.cpp (working copy)
> @@ -200,12 +200,19 @@
>
> /**
> * This was an undocumented API prior to 4.0, but is known to have been used
> by
> - * some IBM applications. This is maintained solely for binary compatibility.
> + * some IBM applications. Therefore this was maintained solely for binary
> + * compatibility.
> *
> + * <new doc TBD>
> + *
> * @return TRUE always.
> */
> -BOOL APIENTRY RexxSetProcessMessages(BOOL onoff)
> +BOOL APIENTRY RexxSetProcessMessages(BOOL turnOn)
> {
> - return TRUE;
> + if ( ! turnOn )
> + {
> + TlsSetValue(SysSemaphore::tlsNoMessageLoopIndex, (LPVOID)1);
> + }
> + return TRUE;
> }
>
> Index: interpreter/platform/windows/SystemInitialization.cpp
> ===================================================================
> --- interpreter/platform/windows/SystemInitialization.cpp (revision
> 5539)
> +++ interpreter/platform/windows/SystemInitialization.cpp (working copy)
> @@ -44,6 +44,7 @@
> #include <stdlib.h> /* Get system, max_path
> etc... */
> #include "RexxCore.h"
> #include "SystemInterpreter.hpp"
> +#include "SysSemaphore.hpp"
>
> /*
> */
> @@ -74,12 +75,23 @@
> {
> if (fdwReason == DLL_PROCESS_ATTACH)
> {
> + // Allocate a thread local storage index
> + SysSemaphore::tlsNoMessageLoopIndex = TlsAlloc();
> + if (SysSemaphore::tlsNoMessageLoopIndex == TLS_OUT_OF_INDEXES)
> + {
> + return FALSE;
> + }
> + SysSemaphore::usingTls = true;
> +
> // perform the interpreter start up
> SystemInterpreter::processStartup(hinstDll);
> }
> else if (fdwReason == DLL_PROCESS_DETACH)
> {
> SystemInterpreter::processShutdown();
> +
> + // Free the thread local storage index
> + TlsFree(SysSemaphore::tlsNoMessageLoopIndex);
> }
> return TRUE;
> }
> Index: common/platform/windows/SysSemaphore.cpp
> ===================================================================
> --- common/platform/windows/SysSemaphore.cpp (revision 5539)
> +++ common/platform/windows/SysSemaphore.cpp (working copy)
> @@ -44,6 +44,9 @@
>
> #include "SysSemaphore.hpp"
>
> +bool SysSemaphore::usingTls = false;
> +DWORD SysSemaphore::tlsNoMessageLoopIndex = 0;
> +
> /**
> * Create a semaphore with potential creation-time
> * initialization.
> Index: common/platform/windows/SysSemaphore.hpp
> ===================================================================
> --- common/platform/windows/SysSemaphore.hpp (revision 5539)
> +++ common/platform/windows/SysSemaphore.hpp (working copy)
> @@ -49,46 +49,8 @@
>
> #include <stdlib.h>
>
> -inline void waitHandle(HANDLE s)
> -{
> - MSG msg = {0};
> +inline void waitHandle(HANDLE s);
>
> - // If already signaled, return.
> - if ( WaitForSingleObject(s, 0) == WAIT_OBJECT_0 )
> - {
> - return;
> - }
> -
> - /** Any thread that creates windows must process messages. A thread that
> - * calls WaitForSingelObject with an infinite timeout risks
> deadlocking the
> - * system. MS's solution for this is to use MsgWaitForMultipleObjects
> to
> - * wait on the object, or a new message arriving in the message
> queue. Some
> - * threads create windows indirectly, an example is COM with
> CoInitialize.
> - * Since we can't know if the current thread has a message queue
> that needs
> - * processing, we use MsgWaitForMultipleObjects.
> - *
> - * Note that MsgWaitForMultipleObjects only returns if a new message is
> - * placed in the queue. PeekMessage alters the state of all messages in
> - * the queue so that they are no longer 'new.' Once PeekMessage
> is called,
> - * all the messages on the queue need to be processed.
> - */
> - do
> - {
> - while ( PeekMessage(&msg, NULL, 0, 0, PM_REMOVE) )
> - {
> - TranslateMessage(&msg);
> - DispatchMessage(&msg);
> -
> - // Check to see if signaled.
> - if ( WaitForSingleObject(s, 0) == WAIT_OBJECT_0 )
> - {
> - return;
> - }
> - }
> - } while ( MsgWaitForMultipleObjects(1, &s, FALSE, INFINITE,
> QS_ALLINPUT) == WAIT_OBJECT_0 + 1 );
> -}
> -
> -
> class SysSemaphore {
> public:
> SysSemaphore() : sem(0) { ; }
> @@ -111,6 +73,9 @@
> inline void reset() { ResetEvent(sem); }
> inline bool posted() { return WaitForSingleObject(sem, 0) != 0; }
>
> + static bool usingTls;
> + static DWORD tlsNoMessageLoopIndex;
> +
> protected:
> HANDLE sem;
> };
> @@ -141,4 +106,69 @@
> protected:
> HANDLE mutexMutex; // the actual mutex
> };
> +
> +
> +/**
> + * Wait for a synchronization object to be in the signaled state.
> + *
> + * Any thread that creates windows must process messages. A thread that
> + * calls WaitForSingelObject with an infinite timeout risks deadlocking the
> + * system. MS's solution for this is to use MsgWaitForMultipleObjects to
> + * wait on the object, or a new message arriving in the message queue. Some
> + * threads create windows indirectly, an example is COM with CoInitialize.
> + * Since we can't know if the current thread has a message queue that needs
> + * processing, we use MsgWaitForMultipleObjects.
> + *
> + * However, with the introduction of the C++ native API in ooRexx 4.0.0, it
> + * became possible for external native libraries to attach to a thread with
> an
> + * active window procedure. If a wait is done on that thread, here,
> + * PeekMessage() causes non-queued messages to be dispatched, and the window
> + * procedure is reentered. This can cause the Rexx program to hang. In
> + * addition, in the one known extension where this problem happens,
> ooDialog,
> + * the messages need to be passed to the dialog manager rather than
> dispatched
> + * directly to the window.
> + *
> + * For this special case, the thread can explicity ask, (through
> + * RexxSetProcessMessages,) that messages are *not* processed during
> this wait.
> + * Thread local storage is used to keep track of a flag signalling this
> case.
> + *
> + * Note that MsgWaitForMultipleObjects only returns if a new message is
> + * placed in the queue. PeekMessage alters the state of all messages in
> + * the queue so that they are no longer 'new.' Once PeekMessage is called,
> + * all the messages on the queue need to be processed.
> + */
> +inline void waitHandle(HANDLE s)
> +{
> + // If already signaled, return.
> + if ( WaitForSingleObject(s, 0) == WAIT_OBJECT_0 )
> + {
> + return;
> + }
> +
> + // If using thread local storage and no message loop is flagged,
> then use WaitForSingleobject()
> + if ( SysSemaphore::usingTls &&
> (DWORD_PTR)TlsGetValue(SysSemaphore::tlsNoMessageLoopIndex) == 1 )
> + {
> + WaitForSingleObject(s, INFINITE);
> + return;
> + }
> +
> + MSG msg = {0};
> +
> + do
> + {
> + while ( PeekMessage(&msg, NULL, 0, 0, PM_REMOVE) )
> + {
> + TranslateMessage(&msg);
> + DispatchMessage(&msg);
> +
> + // Check to see if signaled.
> + if ( WaitForSingleObject(s, 0) == WAIT_OBJECT_0 )
> + {
> + return;
> + }
> + }
> + } while ( MsgWaitForMultipleObjects(1, &s, FALSE, INFINITE,
> QS_ALLINPUT) == WAIT_OBJECT_0 + 1 );
> +}
> +
> +
> #endif
>
> ------------------------------------------------------------------------------
> The Planet: dedicated and managed hosting, cloud storage, colocation
> Stay online with enterprise data centers and the best network in the business
> Choose flexible plans and management services without long-term contracts
> Personal 24x7 support from experience hosting pros just a phone call away.
> http://p.sf.net/sfu/theplanet-com
> _______________________________________________
> Oorexx-devel mailing list
> Oorexx-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/oorexx-devel
>
------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
Oorexx-devel mailing list
Oorexx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/oorexx-devel
