Perl / perl5

šŸŖ The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

win32_msgwait runs forever with non-infinite timeout #7714

Closed p5pRT closed 12 years ago

p5pRT commented 19 years ago

Migrated from rt.perl.org#33096 (status was 'resolved')

Searchable as RT33096$

p5pRT commented 19 years ago

From chris@fatorange.com

Created by chris@fatorange.com

This is a bug report for perl from chris@​fatorange.com\, generated with the help of perlbug 1.35 running under perl v5.8.5.

----------------------------------------------------------------- win32_msgwait will loop forever under the condition that none of the handles becomes ready\, while MsgWaitForMultipleObjects keeps on returning events before the timeout (non-infinite) is ever hit. Just ran into this (timeout​: 1 second) and had a debugger handy - looking at the source\, this should be fairly easy to understand & recreate. The solution would be to decrease the timeout by the amount waited with every run - watch out for DWORD overflows!

Perl Info ``` Flags: category=core severity=critical Site configuration information for perl v5.8.5: Configured by Chris at Tue Dec 7 03:14:58 2004. Summary of my perl5 (revision 5 version 8 subversion 5) configuration: Platform: osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef usethreads=undef use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cl', ccflags ='-nologo -Gf -W3 -MD -Zi -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT -DHAVE_DES_FCRYPT -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -DPERL_MSVCRT_READFIX', optimize='-MD -Zi -DNDEBUG -O1', cppflags='-DWIN32' ccversion='', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -libpath:"c:\si\myperl\lib\CORE" -machine:x86' libpth="C:\Programme\Microsoft Visual Studio .NET\VC7\lib" libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib gnulibc_version='undef' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -libpath:"c:\si\myperl\lib\CORE" -machine:x86' Locally applied patches: @INC for perl v5.8.5: c:/si/myperl/lib c:/si/myperl/site/lib . Environment for perl v5.8.5: HOME (unset) LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=c:\si\myperl\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbe m;C:\PROGRA~1\WINCVS~1.3\CVSNT;C:\Program Files\Darwin Streaming Server PERL_BADLANG (unset) SHELL (unset) ```
p5pRT commented 12 years ago

From @bulk88

On Fri Dec 17 19​:11​:29 2004\, cbecker wrote​:

This is a bug report for perl from chris@​fatorange.com\, generated with the help of perlbug 1.35 running under perl v5.8.5.

----------------------------------------------------------------- win32_msgwait will loop forever under the condition that none of the handles becomes ready\, while MsgWaitForMultipleObjects keeps on returning events before the timeout (non-infinite) is ever hit. Just ran into this (timeout​: 1 second) and had a debugger handy - looking at the source\, this should be fairly easy to understand & recreate. The solution would be to decrease the timeout by the amount waited with every run - watch out for DWORD overflows! Looking at http​://perl5.git.perl.org/perl.git/blob/eb92badec069d2041ebf51c3654ed7e62e7309e9​:/win32/win32.c#l2154 . There is a wall time check and the timer is decreased (actually increased towards expire time) correctly but there is a race with a deadlock (near infinity timeout) if the delay from MsgWaitForMultipleObjects to GetTickCount exceeds 1 ms causing var ticks to go > var timeout. For "while MsgWaitForMultipleObjects keeps on returning events before the timeout (non-infinite) is ever hit"\, if the Perl process is getting a denial-of-service attack through the Windows Message queue from a a different abusive process\, there is nothing Perl can do about it. The PC will be 100% cpu usage anyway from the abusive process. If the messages won't be processed in win32_msgwait they will be processed anyway in assorted perl opcodes in PERL_ASYNC_CHECK()s.

I was able to create a deadlock with a C debugger by doing a Perl "sleep 3;" and creating the delay with breakpoints with a C debugger. Windows running in VM or with a very badly coded driver could cause a context switch that would cause the >= 1 ms delay.

I have a patch nearly complete to prevent the race condition above\, but I realized a different race condition\, the 49.7 day overflow\, see http​://msdn.microsoft.com/en-us/library/windows/desktop/ms724408%28v=vs.85%29.aspx . The only bug the 49.7 day overflow I see creating is\, if you sleep for more than 50 days\, sleep() will return ~ \< 1 day of sleep seconds instead of > 49 days of sleep seconds. So I have 4 choices.

1. Keep GetTickCount and put the 49.7 days of sleep actual seconds slept bug in perlwin32/BUGS AND CAVEATS section as a wont fix. Microsoft considers a likely enough scenario they test apps for the bug\, see http​://msdn.microsoft.com/en-us/subscriptions/aa480483.aspx and search the page for GetTickCount.

2. Replace GetTickCount with QueryPerformanceCounter http​://msdn.microsoft.com/en-us/library/windows/desktop/ms644904%28v=vs.85%29.aspx . The difficulty with this is\, in what psuedo global or real global struct do I keep the frequency ( http​://msdn.microsoft.com/en-us/library/windows/desktop/ms644905%28v=vs.85%29.aspx ) in? It needs to be stored once per Perl DLL loading (non static build)\, or once per perl process startup (static build\, no perl library dll)\, not queried on every ithread interp creation and stored in a my_perl member (duplicate calls to QueryPerformanceFrequency\, I am probably microoptimizing here). Is there a way of storing truly process global (not interp global) data that doesn't have a const initializer or just put it in struct interp_intern and refetch it with QueryPerformanceFrequency as needed\, the more efficient way is simply too difficult with perl's architecture?

3. Replace GetTickCount with GetSystemTimeAsFileTime http​://msdn.microsoft.com/en-us/library/windows/desktop/ms724397%28v=vs.85%29.aspx . I dont know what GetSystemTimeAsFileTime's leap second behavior is.

4. If sleep delta > (49.7 days to seconds/2) assume we overflowed and panic/croak. This doesn't seem like a good idea.

The Perl community's input is needed and appreciated.

p5pRT commented 12 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 12 years ago

From @bulk88

On Mon Aug 20 23​:21​:46 2012\, bulk88 wrote​:

4. If sleep delta > (49.7 days to seconds/2) assume we overflowed and panic/croak. This doesn't seem like a good idea.

The Perl community's input is needed and appreciated.

5. If timeout is > 50% of 2^32 ms or 90% or 99% of 2^32 ms\, croak as invalid timeout. Since posix sleep has no errors\, $! = EINVAL is not needed right? I dont like this idea.

p5pRT commented 12 years ago

From @cpansprout

On Mon Aug 20 23​:21​:46 2012\, bulk88 wrote​:

Is there a way of storing truly process global (not interp global) data that doesn't have a const initializer or just put it in struct interp_intern and refetch it with QueryPerformanceFrequency as needed\, the more efficient way is simply too difficult with perl's architecture?

PL_check is process-global. I donā€™t fully understand how the declarations work\, but it is in perlvars.h​:

#ifdef PERL_GLOBAL_STRUCT PERLVAR(G\, ppaddr\, Perl_ppaddr_t *) /* or opcode.h */ PERLVAR(G\, check\, Perl_check_t *) /* or opcode.h */ PERLVARA(G\, fold_locale\, 256\, unsigned char) /* or perl.h */ #endif

and opcode.h​:

#ifdef PERL_GLOBAL_STRUCT_INIT # define PERL_CHECK_INITED static const Perl_check_t Gcheck[] #else # ifndef PERL_GLOBAL_STRUCT # define PERL_CHECK_INITED EXT Perl_check_t PL_check[] /* or perlvars.h */ # endif #endif

--

Father Chrysostomos

p5pRT commented 12 years ago

From [Unknown Contact. See original ticket]

On Mon Aug 20 23​:21​:46 2012\, bulk88 wrote​:

Is there a way of storing truly process global (not interp global) data that doesn't have a const initializer or just put it in struct interp_intern and refetch it with QueryPerformanceFrequency as needed\, the more efficient way is simply too difficult with perl's architecture?

PL_check is process-global. I donā€™t fully understand how the declarations work\, but it is in perlvars.h​:

#ifdef PERL_GLOBAL_STRUCT PERLVAR(G\, ppaddr\, Perl_ppaddr_t *) /* or opcode.h */ PERLVAR(G\, check\, Perl_check_t *) /* or opcode.h */ PERLVARA(G\, fold_locale\, 256\, unsigned char) /* or perl.h */ #endif

and opcode.h​:

#ifdef PERL_GLOBAL_STRUCT_INIT # define PERL_CHECK_INITED static const Perl_check_t Gcheck[] #else # ifndef PERL_GLOBAL_STRUCT # define PERL_CHECK_INITED EXT Perl_check_t PL_check[] /* or perlvars.h */ # endif #endif

--

Father Chrysostomos

p5pRT commented 12 years ago

From @bulk88

On Mon Aug 20 23​:30​:05 2012\, bulk88 wrote​:

On Mon Aug 20 23​:21​:46 2012\, bulk88 wrote​:

4. If sleep delta > (49.7 days to seconds/2) assume we overflowed and panic/croak. This doesn't seem like a good idea.

The Perl community's input is needed and appreciated.

5. If timeout is > 50% of 2^32 ms or 90% or 99% of 2^32 ms\, croak as invalid timeout. Since posix sleep has no errors\, $! = EINVAL is not needed right? I dont like this idea.

I wrote a C level hooker that hooked GetTickCount from perl517.dll to kernel32.dll. I created an algorithm where the offset from hooked GetTickCount to real GetTickCount grows/compounds by 10 seconds every call to hooked GetTickCount. The sleep time in perl is 5 seconds. Without Visual Studio debugger\, it never deadlocked and was always accurate to less then +/- 1 second. So\, creating an overflow and a deadlock by "(int)(timeout-ticks) \< (int)0" I couldn't reproduce\, on practical terms. The theoretical deadlock I described above might still be possible though.

With Visual Studio debugger\, something interesting happens. The deadlock occurs because the message queue is being spammed (I'm not sure if I am seeing the same message over and over\, or is each time a new message) with a random per per system but not random per process (kindda) message number. The message number is in the Windows allocated range\, using RegisterWindowMessage. Getting the string name on the message number (see google for the GetClipboardFormatName hack) reveals "MSUIM.Msg.Private". 1st page of Google hasn't been very useful in describing what this message is. Callstack for where this MSUIM.Msg.Private message is being fetched over and over is ___________________________________________________________

perl517.dll!win32_async_check(interpreter * my_perl=0x003842a4) Line 2142 C   perl517.dll!win32_msgwait(interpreter * my_perl=0x003842a4\, unsigned long count=0x00000000\, void * * handles=0x00000000\, unsigned long timeout=0x76ade726\, unsigned long * resultp=0x00000000) Line 2177 + 0x9 C   perl517.dll!win32_sleep(unsigned int t=0x0000000a) Line 2342 + 0x19 C   perl517.dll!PerlProcSleep(IPerlProc * piPerl=0x00344510\, unsigned int s=0x0000000a) Line 1659 + 0x9 C++   perl517.dll!Perl_pp_sleep(interpreter * my_perl=0x003842a4) Line 4618 + 0x1a C   perl517.dll!Perl_runops_debug(interpreter * my_perl=0x003842a4) Line 2126 + 0xd C   perl517.dll!S_run_body(interpreter * my_perl=0x003842a4\, long oldscope=0x00000001) Line 2392 + 0xd C   perl517.dll!perl_run(interpreter * my_perl=0x003842a4) Line 2309 + 0xd C   perl517.dll!RunPerl(int argc=0x00000002\, char * * argv=0x00342478\, char * * env=0x00345258) Line 270 + 0x9 C++   perl.exe!main(int argc=0x00000002\, char * * argv=0x00342478\, char * * env=0x00342de0) Line 23 + 0x12 C   perl.exe!mainCRTStartup() Line 398 + 0xe C   kernel32.dll!_BaseProcessStart@​4() + 0x23
_______________________________________________________

I'm not sure how well I will be able to diagnose this since my knowledge with Win32's GUI system is very low. Maybe the actual bug here is\, how does Perl handle getting its Win32 message queue spammed with random messages from the Perl process or other processes that Perl itself doesn't handle since the Perl interp is not a GUI program.

p5pRT commented 12 years ago

From @bulk88

On Tue Sep 04 20​:37​:00 2012\, bulk88 wrote​:

On Mon Aug 20 23​:30​:05 2012\, bulk88 wrote​:

On Mon Aug 20 23​:21​:46 2012\, bulk88 wrote​:

4. If sleep delta > (49.7 days to seconds/2) assume we overflowed and panic/croak. This doesn't seem like a good idea.

The Perl community's input is needed and appreciated.

5. If timeout is > 50% of 2^32 ms or 90% or 99% of 2^32 ms\, croak as invalid timeout. Since posix sleep has no errors\, $! = EINVAL is not needed right? I dont like this idea.

I wrote a C level hooker that hooked GetTickCount from perl517.dll to kernel32.dll. I created an algorithm where the offset from hooked GetTickCount to real GetTickCount grows/compounds by 10 seconds every call to hooked GetTickCount. The sleep time in perl is 5 seconds. Without Visual Studio debugger\, it never deadlocked and was always accurate to less then +/- 1 second. So\, creating an overflow and a deadlock by "(int)(timeout-ticks) \< (int)0" I couldn't reproduce\, on practical terms. The theoretical deadlock I described above might still be possible though.

With Visual Studio debugger\, something interesting happens. The deadlock occurs because the message queue is being spammed (I'm not sure if I am seeing the same message over and over\, or is each time a new message) with a random per per system but not random per process (kindda) message number. The message number is in the Windows allocated range\, using RegisterWindowMessage. Getting the string name on the message number (see google for the GetClipboardFormatName hack) reveals "MSUIM.Msg.Private". 1st page of Google hasn't been very useful in describing what this message is.

I caught a call in the Perl process that registers (or re-registers/fetches the existing????) MSUIM.Msg.Private. KiUserCallbackDispatcher means the kernel decided to run some usermode code in our process. ___ClientLoadLibrary can only be called by the kernel\, it is not called from anywhere inside user32.dll. The string given to RegisterWindowMessageA was "MSUIM.Msg.Private" which is a const string inside MSCTF.dll. ____________________________________________________________

user32.dll!_RegisterWindowMessageA@​4()
  MSCTF.dll!_DllMain@​12() + 0x1dcc
  MSCTF.dll!__DllMainCRTStartup@​12() + 0x48
  ntdll.dll!_LdrpCallInitRoutine@​16() + 0x14
  ntdll.dll!_LdrpRunInitializeRoutines@​4() + 0x205
  ntdll.dll!_LdrpLoadDll@​24() - 0x1b8
  ntdll.dll!_LdrLoadDll@​16() + 0x110
  kernel32.dll!_LoadLibraryExW@​12() + 0xc8
  user32.dll!___ClientLoadLibrary@​4() + 0x32
  ntdll.dll!_KiUserCallbackDispatcher@​12() + 0x13
  user32.dll!_NtUserPeekMessage@​20() + 0xc
  user32.dll!_PeekMessageA@​20() + 0xfb
  perl517.dll!win32_async_check(interpreter * my_perl=0x003842a4) Line 2145 C   perl517.dll!win32_msgwait(interpreter * my_perl=0x003842a4\, unsigned long count=0x00000000\, void * * handles=0x00000000\, unsigned long timeout=0x76866292\, unsigned long * resultp=0x00000000) Line 2177 + 0x9 C   perl517.dll!win32_sleep(unsigned int t=0x0000000a) Line 2342 + 0x19 C   perl517.dll!PerlProcSleep(IPerlProc * piPerl=0x00344510\, unsigned int s=0x0000000a) Line 1659 + 0x9 C++   perl517.dll!Perl_pp_sleep(interpreter * my_perl=0x003842a4) Line 4618 + 0x1a C   perl517.dll!Perl_runops_debug(interpreter * my_perl=0x003842a4) Line 2126 + 0xd C   perl517.dll!S_run_body(interpreter * my_perl=0x003842a4\, long oldscope=0x00000001) Line 2392 + 0xd C   perl517.dll!perl_run(interpreter * my_perl=0x003842a4) Line 2309 + 0xd C   perl517.dll!RunPerl(int argc=0x00000002\, char * * argv=0x00342478\, char * * env=0x00345258) Line 270 + 0x9 C++   perl.exe!main(int argc=0x00000002\, char * * argv=0x00342478\, char * * env=0x00342de0) Line 23 + 0x12 C   perl.exe!mainCRTStartup() Line 398 + 0xe C   kernel32.dll!_BaseProcessStart@​4() + 0x23
_________________________________________________________

p5pRT commented 12 years ago

From @bulk88

The problem with VS Debugger is it loads msctf.dll into the Perl process through an kernel mechanism in win32k.sys (kernel driver side of user32.dll) I don't really understand (callstack was posted above). The VS IDE app (devenv.exe) also loads msctf.dll. I think the msctf.dll in the VS debugger is trying to do IPC with the child's msctf.dll instance. I think\, if a break point in VS debugger is set between the PeekMessage on NULL hwnd in win32_async_check\, and MsgWaitForMultipleObjects\, extra messages get added to the thread's message queue\, so MsgWaitForMultipleObjects trips immediately as [msg queue] handle ready and win32_async_check runs again hitting the breakpoint that generates a new message and MsgWaitForMultipleObjects trips again in a infinite look since MsgWaitForMultipleObjects will never timeout since the msg queue handle is already ready.

According to my disassembly of msctf.dll\, function "OnForegroundChange" in msctf.dll is the source of the message (message=MSUIM.Msg.Private\, wParam=1\, lParam=0). I tried to debug VS debugger but freezes and deadlocks with VS debugger debugging another VS debugger\, and in WinDbg OnForegroundChange was hit ever time I tried to "go" the VS debugger process\, made it impossible for me to try to get a callstack from VS debugger\, so I give up on that. Since after a breakpoint the Perl console window jumps to the front of VS IDE\, my theory is plausible. So that part of this I dont think can be fixed. "that part" is setting a breakpoint between PeekMessage and MsgWaitForMultipleObjects.

From the knowledge above\, a hang from GetTickCount overflow I was able to reproduce on blead 5.17 perl using 2 scripts that are attached.

p5pRT commented 12 years ago

From @bulk88

sendthread.pl

p5pRT commented 12 years ago

From @bulk88

hook.pl

p5pRT commented 12 years ago

From @bulk88

On Fri Sep 07 16​:00​:03 2012\, bulk88 wrote​:

The problem with VS Debugger is it loads msctf.dll into the Perl process ...

Egh\, again I didn't forward my posts to P5P list from RT interface.

I wrote a patch that solves the overflow and underflow issues I identified while investigating 33096. I couldn't decide whether to put a "Perl's sleep on Windows won't work correctly if you specify more than 49 days of seconds as an argument to sleep" know problem in README.win32 or in perlport.pod so someone else do that if appropriate. The far fetched way to fix this VS debugger bug would be to completely remove using the message queue in Perl. Instead use APC timers and QueueUserAPC for inter-fork/inter-ithread communication and timers. Or more adventurously\, since both APCs and the message queue are OS thread locked on Windows\, use an IOCP\, with APCs timers delivered to a kernel32 threadpool (win2k or newer) thread which then posts the timer to an IOCP. Any OS thread can subscribe and listen to an IOCP queue. Moving Perl interps between OS threads isn't done by Perl\, and is an very rare thing done in general (I've seen working examples of it Perlmonks).

I'm not sure if this bug should remain open (more than 49 days of seconds passed to sleep)\, rejected (it is VS Debugger's bug)\, or resolved.

p5pRT commented 12 years ago

From @bulk88

0001-perl-33096-fix-over-underflow-issues-in-win32_msgwai.patch ```diff From 656effa89f7bb0559f15061ebba096665eb5a3a6 Mon Sep 17 00:00:00 2001 From: Daniel Dragan Date: Sat, 8 Sep 2012 03:03:24 -0400 Subject: [PATCH] [perl #33096] fix over/underflow issues in win32_msgwait This commit does not completely fix 33096, since the real problem is that VS IDE's debugger causes a Windows message to be sent to perl on each breakpoint. Depending on where the breakpoint was set. The BP may make it impossible to exit the loop because of the Visual Studio IDE Debugger's design. Various overflow and underflow issues were fixed in win32_msgwait. Specifically, the time count rolling forwards through zero (GetSystemTimeAsFileTime), and the time count running ahead of the end time (rolling backwards through zero) were fixed ("<=" check). --- pod/perldelta.pod | 5 +++++ win32/win32.c | 45 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index d9b2cc6..19009ff 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -349,6 +349,11 @@ files in F and F are best summarized in L. =item * +On Windows, a rare race condition that would lead to L +taking more time than requested, and upto a hang has been fixed [perl #33096]. + +=item * + XXX =back diff --git a/win32/win32.c b/win32/win32.c index 78b55f6..93c2102 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -2154,13 +2154,32 @@ DllExport DWORD win32_msgwait(pTHX_ DWORD count, LPHANDLE handles, DWORD timeout, LPDWORD resultp) { /* We may need several goes at this - so compute when we stop */ - DWORD ticks = 0; + unsigned __int64 ticks = 0; + unsigned __int64 endtime = timeout; if (timeout != INFINITE) { - ticks = GetTickCount(); - timeout += ticks; - } - while (1) { - DWORD result = MsgWaitForMultipleObjects(count,handles,FALSE,timeout-ticks, QS_POSTMESSAGE|QS_TIMER|QS_SENDMESSAGE); + GetSystemTimeAsFileTime((LPFILETIME)&ticks); + ticks /= 10000; + endtime += ticks; + } + /* This was a race condition. Do not let a non INFINITE timeout to + * MsgWaitForMultipleObjects roll under 0 creating a near + * infinity/~(UINT32)0 timeout which will appear as a deadlock to the + * user who did a CORE perl function with a non infinity timeout, + * sleep for example. This is 64 to 32 truncation minefield. + * + * This scenario can only be created if the timespan from the return of + * MsgWaitForMultipleObjects to GetTickCount exceeds 1 ms. To generate + * the scenario, manual breakpoints in a C debugger are required, or a + * context switch occured in win32_async_check in PeekMessage, or random + * messages are delivered to the *thread* message queue of the Perl thread + * from another process (msctf.dll doing IPC among its instances, VS debugger + * causes msctf.dll to be loaded into Perl by kernel) + */ + while (ticks <= endtime) { + /* if timeout's type is lengthened, remember to split 64b timeout + * into multiple non-infinity runs of MWFMO */ + DWORD result = MsgWaitForMultipleObjects(count,handles,FALSE,(DWORD)(endtime-ticks) + , QS_POSTMESSAGE|QS_TIMER|QS_SENDMESSAGE); if (resultp) *resultp = result; if (result == WAIT_TIMEOUT) { @@ -2170,8 +2189,9 @@ win32_msgwait(pTHX_ DWORD count, LPHANDLE handles, DWORD timeout, LPDWORD result return 0; } if (timeout != INFINITE) { - ticks = GetTickCount(); - } + GetSystemTimeAsFileTime((LPFILETIME)&ticks); + ticks /= 10000; + } if (result == WAIT_OBJECT_0 + count) { /* Message has arrived - check it */ (void)win32_async_check(aTHX); @@ -2181,10 +2201,13 @@ win32_msgwait(pTHX_ DWORD count, LPHANDLE handles, DWORD timeout, LPDWORD result break; } } - /* compute time left to wait */ - ticks = timeout - ticks; /* If we are past the end say zero */ - return (ticks > 0) ? ticks : 0; + if(!ticks || ticks > endtime) + return 0; + /* compute time left to wait */ + ticks = endtime - ticks; + /*if more ms than DWORD, then return max DWORD*/ + return ticks <= UINT_MAX ? (DWORD)ticks:UINT_MAX; } int -- 1.7.9.msysgit.0 ```
p5pRT commented 12 years ago

From @bulk88

I also never figured out if this line http​://perl5.git.perl.org/perl.git/blob/2d2826733b14efb7509c9c0c28d27bca6f31d681​:/win32/win32.c#l2142 is correct or not. I thought perhaps there are multiple messages on the queue and the PeekMessage only unfreshens the 1st one and thats why MWFMO kept tripping on a handle. I tried putting the PeekMessage in a loop to drain the queue\, but I believe due to the PM_NOREMOVE flag\, the PeekMessage never failed and never exited the loop. Should perl is "processing" and "dispatching" the *thread queue* messages at that line or not? IDK enough to comment.

p5pRT commented 12 years ago

From @b2gills

On Sat\, Sep 8\, 2012 at 2​:33 AM\, bulk 88 via RT \perlbug\-followup@&#8203;perl\.org wrote​:

I also never figured out if this line http​://perl5.git.perl.org/perl.git/blob/2d2826733b14efb7509c9c0c28d27bca6f31d681​:/win32/win32.c#l2142 is correct or not. I thought perhaps there are multiple messages on the queue and the PeekMessage only unfreshens the 1st one and thats why MWFMO kept tripping on a handle. I tried putting the PeekMessage in a loop to drain the queue\, but I believe due to the PM_NOREMOVE flag\, the PeekMessage never failed and never exited the loop. Should perl is "processing" and "dispatching" the *thread queue* messages at that line or not? IDK enough to comment.

PeekMessage without PM_REMOVE doesn't remove messages from the queue. That is perhaps the main reason to use PeekMessage instead of GetMessage. The only other reason is to use PM_NOYIELD

https://www.google.com/search?q=site%3Amsdn.microsoft.com+PeekMessage&btnI=1 https://www.google.com/search?q=site%3Amsdn.microsoft.com+GetMessage&btnI=1

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=33096

p5pRT commented 12 years ago

From @bulk88

On Sat Sep 08 00​:20​:25 2012\, bulk88 wrote​:

I wrote a patch that solves the overflow and underflow issues I identified while investigating 33096. I couldn't decide whether to put a "Perl's sleep on Windows won't work correctly if you specify more than 49 days of seconds as an argument to sleep" know problem in README.win32 or in perlport.pod so someone else do that if appropriate.

Can someone please review this patch?

p5pRT commented 12 years ago

From @steve-m-hay

bulk 88 via RT wrote on 2012-09-13​:

On Sat Sep 08 00​:20​:25 2012\, bulk88 wrote​:

I wrote a patch that solves the overflow and underflow issues I identified while investigating 33096. I couldn't decide whether to put a "Perl's sleep on Windows won't work correctly if you specify more than 49 days of seconds as an argument to sleep" know problem in README.win32 or in perlport.pod so someone else do that if appropriate.

Can someone please review this patch?

I hope to have a look very soon...

(Apologies for the slow responses to all your investigations and patches\, btw. They really are appreciated\, but Windows porters are thin on the ground so it can take a while to get round to them. I have numerous others sat in my Inbox too; they haven't been forgotten.)

p5pRT commented 12 years ago

From @steve-m-hay

On Thu Sep 13 10​:11​:32 2012\, Steve.Hay@​verosoftware.com wrote​:

bulk 88 via RT wrote on 2012-09-13​:

Can someone please review this patch?

I hope to have a look very soon...

I've applied the patch locally and have done some testing.

I reproduced the hang using your test scripts and the original code. With your patch in place the test scripts no longer hang\, but I do ocasionally see this message output in the console just before hook.pl exits​:

bad time t1 1347611551.84041 t2 1347611561.84291 10.0025000572205 at hook.pl line 53.

Is this anything to be concerned about?

PS​: I also tried out your Win32​::API 0.71 in the process in order to run your test script\, and found that two tests failed in it with a perl built with ithreads but USE_IMP_SYS undefined because such a perl has ithreads but no fork. The attached patch fixes those failures by doing similar tests as are done in other modules for this.

p5pRT commented 12 years ago

From @steve-m-hay

nofork.patch ```diff diff -ruN bulk88-perl5-win32-api-70da72a.orig/Callback/t/iat.t bulk88-perl5-win32-api-70da72a/Callback/t/iat.t --- bulk88-perl5-win32-api-70da72a.orig/Callback/t/iat.t 2012-09-03 01:32:52.000000000 +0100 +++ bulk88-perl5-win32-api-70da72a/Callback/t/iat.t 2012-09-14 09:09:54.494412500 +0100 @@ -86,6 +86,10 @@ Win32::GetProcAddress(Win32::LoadLibrary("kernel32.dll"), 'QueryPerformanceCounter'), "GetOriginalFunctionPtr returns real QPC"); +my $can_fork = $Config{d_fork} || $Config{d_pseudofork} || + (($^O eq 'MSWin32' || $^O eq 'NetWare') and + $Config{useithreads} and $Config{ccflags} =~ /-DPERL_IMPLICIT_SYS/); + SKIP: { Win32::API::Type->typedef('PRTL_PROCESS_MODULES', 'char *'); my $LdrQueryProcessModuleInformation = @@ -93,8 +97,8 @@ "NTSTATUS NTAPI LdrQueryProcessModuleInformation(". "PRTL_PROCESS_MODULES ModuleInformation, ULONG Size, PULONG ReturnedSize)"); - skip("This Perl doesn't have ithreads and/or this Windows OS doesn't have " - ."LdrQueryProcessModuleInformation", 6) if ! $Config{'useithreads'} + skip("This Perl doesn't have fork and/or this Windows OS doesn't have " + ."LdrQueryProcessModuleInformation", 6) if ! $can_fork || ! $LdrQueryProcessModuleInformation; #Native API changed, thats ok is(GetAPITestDLLLoadCount($LdrQueryProcessModuleInformation), 1, "DLL load count is 1 before fork"); @@ -213,4 +217,4 @@ return $_->{LoadCount}; } } -} \ No newline at end of file +} diff -ruN bulk88-perl5-win32-api-70da72a.orig/Callback/t/ithreads.t bulk88-perl5-win32-api-70da72a/Callback/t/ithreads.t --- bulk88-perl5-win32-api-70da72a.orig/Callback/t/ithreads.t 2012-09-03 01:32:52.000000000 +0100 +++ bulk88-perl5-win32-api-70da72a/Callback/t/ithreads.t 2012-09-14 09:10:33.786912500 +0100 @@ -13,8 +13,12 @@ #HeapBlock class is not public API +my $can_fork = $Config{d_fork} || $Config{d_pseudofork} || + (($^O eq 'MSWin32' || $^O eq 'NetWare') and + $Config{useithreads} and $Config{ccflags} =~ /-DPERL_IMPLICIT_SYS/); + SKIP: { - skip("This Perl doesn't have ithreads", 1) if ! $Config{'useithreads'}; + skip("This Perl doesn't have fork", 1) if ! $can_fork; #50 megs should be enough to force a VirtualAlloc and a VirtualFree my $ptrobj = new Win32::API::Callback::HeapBlock 5000000; my $pid = fork(); ```
p5pRT commented 12 years ago

From @bulk88

On Fri Sep 14 01​:36​:46 2012\, shay wrote​:

I've applied the patch locally and have done some testing.

I reproduced the hang using your test scripts and the original code. With your patch in place the test scripts no longer hang\, but I do ocasionally see this message output in the console just before hook.pl exits​:

bad time t1 1347611551.84041 t2 1347611561.84291 10.0025000572205 at hook.pl line 53.

Is this anything to be concerned about? I dont think so. I forgot to add +/- 1 second tolerance to the script since this is FP math after all. Perl's tests have tolerances\, see http​://perl5.git.perl.org/perl.git/blob/b71c54b89aa7c965d23cc7cfdb7dff179d8f4163​:/t/op/alarm.t#l35 and http​://perl5.git.perl.org/perl.git/blob/b71c54b89aa7c965d23cc7cfdb7dff179d8f4163​:/t/op/sleep.t . The 0.002 is 2 ms which is a reasonable overshoot or FP error.

PS​: I also tried out your Win32​::API 0.71 in the process in order to run your test script\, and found that two tests failed in it with a perl built with ithreads but USE_IMP_SYS undefined because such a perl has ithreads but no fork. The attached patch fixes those failures by doing similar tests as are done in other modules for this.

Thanks for the fork detection patch to Win32​::API.

If there is something wrong about my assumptions of C integer math/integer casting in the patch tell me. C isn't my day job.

I changed from GetTickCount (32 bits) to GetSystemTimeAsFileTime (64 bits) since it is the easiest way of dealing with 0 rollover. The 2 functions are siblings under the hood\, both are polled from a KUSER_SHARED_DATA by kernel32. 49 day limit remains. However strange\, someone with Perl might try to write a watchdog with a 6 month timer using sleep to reboot the system or kill and restart a process. I couldn't decide if\, and where\, (README.win32 or perlport.pod) a sentence about the 49 day limit of seconds to Win32 Perl sleep should go.

I also created a GetSystemTimeAsFileTime equivalent (plus 1 second tolerance added) of hook.pl called hook64.pl. hook.pl doesn't work after the patch since Perl doesn't import GetTickCount anymore. GetSystemTimeAsFileTime is used by post patch win32_msgwait and pre-patch win32_gettimeofday. hook64.pl uses sendthread.pl just like hook.pl\, and hook64.pl didn't hang for me. hook64.pl won't work on a pre-patch win32_msgwait and I never wrote a overflow/rollunder GetSystemTimeAsFileTime win32_msgwait.

p5pRT commented 12 years ago

From @bulk88

hook64.pl

p5pRT commented 12 years ago

From @bulk88

On Fri Sep 14 18​:33​:39 2012\, bulk88 wrote​:

On Fri Sep 14 01​:36​:46 2012\, shay wrote​:

I've applied the patch locally and have done some testing.

I reproduced the hang using your test scripts and the original code. With your patch in place the test scripts no longer hang\, but I do ocasionally see this message output in the console just before hook.pl exits​:

bad time t1 1347611551.84041 t2 1347611561.84291 10.0025000572205 at hook.pl line 53.

Is this anything to be concerned about? I dont think so. I forgot to add +/- 1 second tolerance to the script since this is FP math after all. Perl's tests have tolerances\, see

http​://perl5.git.perl.org/perl.git/blob/b71c54b89aa7c965d23cc7cfdb7dff179d8f4163​:/t/op/alarm.t#l35 and

http​://perl5.git.perl.org/perl.git/blob/b71c54b89aa7c965d23cc7cfdb7dff179d8f4163​:/t/op/sleep.t . The 0.002 is 2 ms which is a reasonable overshoot or FP error.

PS​: I also tried out your Win32​::API 0.71 in the process in order to run your test script\, and found that two tests failed in it with a perl built with ithreads but USE_IMP_SYS undefined because such a perl has ithreads but no fork. The attached patch fixes those failures by doing similar tests as are done in other modules for this.

Thanks for the fork detection patch to Win32​::API.

If there is something wrong about my assumptions of C integer math/integer casting in the patch tell me. C isn't my day job.

I changed from GetTickCount (32 bits) to GetSystemTimeAsFileTime (64 bits) since it is the easiest way of dealing with 0 rollover. The 2 functions are siblings under the hood\, both are polled from a KUSER_SHARED_DATA by kernel32. 49 day limit remains. However strange\, someone with Perl might try to write a watchdog with a 6 month timer using sleep to reboot the system or kill and restart a process. I couldn't decide if\, and where\, (README.win32 or perlport.pod) a sentence about the 49 day limit of seconds to Win32 Perl sleep should go.

I also created a GetSystemTimeAsFileTime equivalent (plus 1 second tolerance added) of hook.pl called hook64.pl. hook.pl doesn't work after the patch since Perl doesn't import GetTickCount anymore. GetSystemTimeAsFileTime is used by post patch win32_msgwait and pre-patch win32_gettimeofday. hook64.pl uses sendthread.pl just like hook.pl\, and hook64.pl didn't hang for me. hook64.pl won't work on a pre-patch win32_msgwait and I never wrote a overflow/rollunder GetSystemTimeAsFileTime win32_msgwait.

RT web post didn't forward to P5P so I am forwarding.

p5pRT commented 12 years ago

From @steve-m-hay

On Sat Sep 15 22​:56​:21 2012\, bulk88 wrote​:

On Fri Sep 14 18​:33​:39 2012\, bulk88 wrote​:

On Fri Sep 14 01​:36​:46 2012\, shay wrote​:

I've applied the patch locally and have done some testing.

I reproduced the hang using your test scripts and the original code. With your patch in place the test scripts no longer hang\, but I do ocasionally see this message output in the console just before hook.pl exits​:

bad time t1 1347611551.84041 t2 1347611561.84291 10.0025000572205 at hook.pl line 53.

Is this anything to be concerned about? I dont think so. I forgot to add +/- 1 second tolerance to the script since this is FP math after all. Perl's tests have tolerances\, see

http​://perl5.git.perl.org/perl.git/blob/b71c54b89aa7c965d23cc7cfdb7dff179d8f4163​:/t/op/alarm.t#l35

and

http​://perl5.git.perl.org/perl.git/blob/b71c54b89aa7c965d23cc7cfdb7dff179d8f4163​:/t/op/sleep.t

. The 0.002 is 2 ms which is a reasonable overshoot or FP error.

Ok.

[...]

If there is something wrong about my assumptions of C integer math/integer casting in the patch tell me. C isn't my day job.

I think the use of (LPFILETIME)&ticks as the argument to GetSystemTimeAsFileTime() in two places is wrong​: treating an __int64 as a FILETIME or vice versa can cause alignment issues.

It would be safer to use a real FILETIME and then convert it to __int64 using

  __int64 qw = ft->dwHighDateTime;   qw \<\<= 32;   qw |= ft->dwLowDateTime;

(see filetime_to_clock()) or else via ULARGE_INTEGER\, or better yet use the FT_t union trick from win32_gettimeofday() :-)

I changed from GetTickCount (32 bits) to GetSystemTimeAsFileTime (64 bits) since it is the easiest way of dealing with 0 rollover. The 2 functions are siblings under the hood\, both are polled from a KUSER_SHARED_DATA by kernel32. 49 day limit remains. However strange\, someone with Perl might try to write a watchdog with a 6 month timer using sleep to reboot the system or kill and restart a process. I couldn't decide if\, and where\, (README.win32 or perlport.pod) a sentence about the 49 day limit of seconds to Win32 Perl sleep should go.

I think a note in perlport.pod would be fine.

I also created a GetSystemTimeAsFileTime equivalent (plus 1 second tolerance added) of hook.pl called hook64.pl. hook.pl doesn't work after the patch since Perl doesn't import GetTickCount anymore. GetSystemTimeAsFileTime is used by post patch win32_msgwait and pre-patch win32_gettimeofday. hook64.pl uses sendthread.pl just like hook.pl\, and hook64.pl didn't hang for me. hook64.pl won't work on a pre-patch win32_msgwait and I never wrote a overflow/rollunder GetSystemTimeAsFileTime win32_msgwait.

The new hook64.pl is working fine for me with your patch applied. I'm happy to apply it if you fix up the FILETIME/__int64 issue. Please can you also correct the reference to GetTickCount in the comment (it should refer to GetSystemTimeAsFileTime instead now) and fix up the tabbing/indentation\, which is wrong on both "ticks /= 10000;" lines and the } after the second of those.

Thanks.

p5pRT commented 12 years ago

From @bulk88

On Sun Sep 16 09​:23​:33 2012\, shay wrote​:

I think the use of (LPFILETIME)&ticks as the argument to GetSystemTimeAsFileTime() in two places is wrong​: treating an __int64 as a FILETIME or vice versa can cause alignment issues. .................................... (see filetime_to_clock()) or else via ULARGE_INTEGER\, or better yet use the FT_t union trick from win32_gettimeofday() :-)

I changed the patch to use a FT_t. An auto FILETIME is aligned to atleast 4\, a __int64 is aligned to atleast 8. The FILETIME to __int64 cast is a problem\, the other direction isn't a problem IMO.

I think a note in perlport.pod would be fine.

I'm thinking rather than a note in perlport.pod\, should I add a croak instead? Invented a new message for perldiag\, or reuse an existing one? In existing ones\, the best (but still not really relevant/bad choice) ones I thought were _________________________________________________ Number too long

(F) Perl limits the representation of decimal numbers in programs to about 250 characters. You've exceeded that length. Future versions of Perl are likely to eliminate this arbitrary limitation. In the meantime\, try using scientific notation (e.g. "1e6" instead of "1_000_000").

Integer overflow in %s number

(W overflow) The hexadecimal\, octal or binary number you have specified either as a literal or as an argument to hex() or oct() is too big for your architecture\, and has been converted to a floating point number. On a 32-bit architecture the largest hexadecimal\, octal or binary number representable without overflow is 0xFFFFFFFF\, 037777777777\, or 0b11111111111111111111111111111111 respectively. Note that Perl transparently promotes all numbers to a floating point representation internally--subject to loss of precision errors in subsequent operations.

Type of arg %d to &CORE​::%s must be %s

(F) The subroutine in question in the CORE package requires its argument to be a hard reference to data of the specified type. Overloading is ignored\, so a reference to an object that is not the specified type\, but nonetheless has overloading to handle it\, will still not be accepted. _____________________________________________________

The croak condition would be added to win32_sleep as "t > (UINT_MAX/1000)". For the choice of a new croak message "sleep(%u) too large for your platform" (wording from existing "gmtime(%f) too large") then specific. Unless someone can think of a better worded message. So perlport note or add a croak and a new perldiag message? I do realize the best fix is to make win32_sleep do a loop to be compatible with POSIX sleep but I'd rather just get this bug done with. I did read the POSIX standard for sleep (as a separate problem/ticket\, from a glance\, I dont think that a Win32 Perl alarm will cause Perl sleep to end early\, alarm.t seems to have been specifically written never uses Perl sleep in the same Perl process that is doing the Perl alarm). If you think it is important\, I will try to make win32_sleep POSIX sleep complaint by looping to remove the 49 day limit. If I do the perlport fix instead of the croak\, should I also add that Win32 Perl sleep will not return early after an Win32 Perl alarm signal fires?

and fix up the tabbing/indentation\, which is wrong on both "ticks /= 10000;" lines and the } after the second of those.

What is the standard for Perl\, a tab or spaces for indentation? Looking at win32.c it seems random line by line. Sometimes I see a line starting with a tab but all subsequent indents are spaces.

I will upload another patch after the issues brought up by you and me are resolved.

p5pRT commented 12 years ago

From @steve-m-hay

bulk 88 via RT wrote on 2012-09-17​:

On Sun Sep 16 09​:23​:33 2012\, shay wrote​:

I think a note in perlport.pod would be fine.

I'm thinking rather than a note in perlport.pod\, should I add a croak instead? Invented a new message for perldiag\, or reuse an existing one? [...] The croak condition would be added to win32_sleep as "t > (UINT_MAX/1000)". For the choice of a new croak message "sleep(%u) too large for your platform" (wording from existing "gmtime(%f) too large") then specific. Unless someone can think of a better worded message. So perlport note or add a croak and a new perldiag message? I do realize the best fix is to make win32_sleep do a loop to be compatible with POSIX sleep but I'd rather just get this bug done

I don't think a croak would be very friendly for the user. The gmtime message which you refer to is a warning not a croak (that's what the "(W)" in perldiag signifies). A similar new *warning* with the wording you suggested would be fine\, but I'd still be inclined to add a note in perlport too.

with. I did read the POSIX standard for sleep (as a separate problem/ticket\, from a glance\, I dont think that a Win32 Perl alarm will cause Perl sleep to end early\, alarm.t seems to have been specifically written never uses Perl sleep in the same Perl process that is doing the Perl alarm). If you think it is important\, I will try to make win32_sleep POSIX sleep complaint by looping to remove the 49 day limit. If I do the perlport fix instead of the croak\, should I also add that Win32 Perl sleep will not return early after an Win32 Perl alarm signal fires?

I think the fact that alarm won't interrupt a sleep is already covered by the perlport entry for alarm​:

"Emulated using timers that must be explicitly polled whenever Perl wants to dispatch "safe signals" and therefore cannot interrupt blocking system calls. (Win32)"

I see no need to remove the 49 day limit from sleep as part of this bug fix. The (existing) limitation will be clearly documented now\, so anyone encountering the problem in the future will know that it's a known issue\, and can try fixing it if they wish.

and fix up the tabbing/indentation\, which is wrong on both "ticks /= 10000;" lines and the } after the second of those.

What is the standard for Perl\, a tab or spaces for indentation? Looking at win32.c it seems random line by line. Sometimes I see a line starting with a tab but all subsequent indents are spaces.

The existing tabbing isn't perfect by any means\, but I at least try to add new code according to the style specified in perlhack​:

* 8-wide tabs (no exceptions!) * 4-wide indents for code\, 2-wide indents for nested CPP #defines

p5pRT commented 12 years ago

From @jandubois

On Mon\, 17 Sep 2012\, Steve Hay wrote​:

I think the fact that alarm won't interrupt a sleep is already covered by the perlport entry for alarm​:

Sorry\, haven't found time yet to read this thread\, but alarm() emulation on Windows is supposed to interrupt sleep(). A quick check confirms that it still works​:

C​:\>perl -E"$SIG{ALRM}=sub{die};say time;alarm 2;eval{sleep 10};say time" 1347909521 1347909523

sleep() is also emulated\, so doesn't count as a "blocking system call". Maybe this should be documented in perlport.pod.

Cheers\, -Jan

p5pRT commented 12 years ago

From @steve-m-hay

On Mon Sep 17 12​:22​:28 2012\, jdb wrote​:

On Mon\, 17 Sep 2012\, Steve Hay wrote​:

I think the fact that alarm won't interrupt a sleep is already covered by the perlport entry for alarm​:

Sorry\, haven't found time yet to read this thread\, but alarm() emulation on Windows is supposed to interrupt sleep(). A quick check confirms that it still works​:

C​:\>perl -E"$SIG{ALRM}=sub{die};say time;alarm 2;eval{sleep 10};say time" 1347909521 1347909523

sleep() is also emulated\, so doesn't count as a "blocking system call". Maybe this should be documented in perlport.pod.

Thanks\, I hadn't realized that. Would something like this be suitable then\, or were you thinking of expanding the alarm() entry instead?

Inline Patch ```diff diff --git a/pod/perlport.pod b/pod/perlport.pod index a37bc7c..023a8c9 100644 --- a/pod/perlport.pod +++ b/pod/perlport.pod @@ -1921,6 +1921,11 @@ Not implemented. (S) Not implemented. (Win32, VMS, S, VOS) +=item sleep + +Emulated using synchronization functions such that it can be +interrupted by alarm(). (Win32) + =item sockatmark A relatively recent addition to socket functions, may not ```
p5pRT commented 12 years ago

From @bulk88

Sorry\, haven't found time yet to read this thread\, but alarm() emulation on Windows is supposed to interrupt sleep(). A quick check confirms that it still works​:

C​:\>perl -E"$SIG{ALRM}=sub{die};say time;alarm 2;eval{sleep 10};say time" 1347909521 1347909523

sleep() is also emulated\, so doesn't count as a "blocking system call". Maybe this should be documented in perlport.pod.

Cheers\, -Jant

But sleep was interrupted by the signal\, the sleep when *poof* with the longjmp/die. The was only possible with eval and die\, if you remove them and want Perl sleep to return early posix style on win32 it wont work.

_____________________________________ C​:\p517\perl>perl -E "$SIG{ALRM}=sub{0;};say time;alarm 2;sleep 10;say time" 1347955780 1347955790

C​:\p517\perl>r __________________________________

I also attached the new patch to this post.

p5pRT commented 12 years ago

From @bulk88

0001-perl-33096-fix-over-underflow-issues-in-win32_msgwai.patch ```diff From ccb927cbd4253b51de2e5a90d2b03bc8afe37dce Mon Sep 17 00:00:00 2001 From: Daniel Dragan Date: Tue, 18 Sep 2012 04:17:21 -0400 Subject: [PATCH] [perl #33096] fix over/underflow issues in win32_msgwait This commit does not completely fix 33096, since the real problem is that VS IDE's debugger causes a Windows message to be sent to perl on each breakpoint. Depending on where the breakpoint was set. The BP may make it impossible to exit the loop because of the Visual Studio IDE Debugger's design. Various overflow and underflow issues were fixed in win32_msgwait. Specifically, the time count rolling forwards through zero (GetSystemTimeAsFileTime), and the time count running ahead of the end time (rolling backwards through zero) were fixed ("<=" check). --- pod/perldelta.pod | 3 +++ pod/perlport.pod | 6 ++++++ win32/win32.c | 45 ++++++++++++++++++++++++++++++++++----------- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index c1b7be1..d20f95a 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -629,6 +629,9 @@ Fixed a problem where perl could crash while cleaning up threads (including the main thread) in threaded debugging builds on Win32 and possibly other platforms [perl #114496]. +A rare race condition that would lead to L +taking more time than requested, and upto a hang has been fixed [perl #33096]. + =item Solaris In Configure, avoid running sed commands with flags not supported on Solaris. diff --git a/pod/perlport.pod b/pod/perlport.pod index a37bc7c..839468b 100644 --- a/pod/perlport.pod +++ b/pod/perlport.pod @@ -1921,6 +1921,12 @@ Not implemented. (S) Not implemented. (Win32, VMS, S, VOS) +=item sleep + +On Win32 C is limited to a maximum of 4294967 seconds, approximately 49 +days, and also signals do not cause C to return early once a signal +fires. C will always wait the full time period before returning. + =item sockatmark A relatively recent addition to socket functions, may not diff --git a/win32/win32.c b/win32/win32.c index c2ad58f..47127d9 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -2217,13 +2217,32 @@ DllExport DWORD win32_msgwait(pTHX_ DWORD count, LPHANDLE handles, DWORD timeout, LPDWORD resultp) { /* We may need several goes at this - so compute when we stop */ - DWORD ticks = 0; + FT_t ticks = {0}; + unsigned __int64 endtime = timeout; if (timeout != INFINITE) { - ticks = GetTickCount(); - timeout += ticks; - } - while (1) { - DWORD result = MsgWaitForMultipleObjects(count,handles,FALSE,timeout-ticks, QS_POSTMESSAGE|QS_TIMER|QS_SENDMESSAGE); + GetSystemTimeAsFileTime(&ticks.ft_val); + ticks.ft_i64 /= 10000; + endtime += ticks.ft_i64; + } + /* This was a race condition. Do not let a non INFINITE timeout to + * MsgWaitForMultipleObjects roll under 0 creating a near + * infinity/~(UINT32)0 timeout which will appear as a deadlock to the + * user who did a CORE perl function with a non infinity timeout, + * sleep for example. This is 64 to 32 truncation minefield. + * + * This scenario can only be created if the timespan from the return of + * MsgWaitForMultipleObjects to GetSystemTimeAsFileTime exceeds 1 ms. To + * generate the scenario, manual breakpoints in a C debugger are required, + * or a context switch occured in win32_async_check in PeekMessage, or random + * messages are delivered to the *thread* message queue of the Perl thread + * from another process (msctf.dll doing IPC among its instances, VS debugger + * causes msctf.dll to be loaded into Perl by kernel), see [perl #33096]. + */ + while (ticks.ft_i64 <= endtime) { + /* if timeout's type is lengthened, remember to split 64b timeout + * into multiple non-infinity runs of MWFMO */ + DWORD result = MsgWaitForMultipleObjects(count,handles,FALSE,(DWORD)(endtime-ticks.ft_i64) + , QS_POSTMESSAGE|QS_TIMER|QS_SENDMESSAGE); if (resultp) *resultp = result; if (result == WAIT_TIMEOUT) { @@ -2233,8 +2252,9 @@ win32_msgwait(pTHX_ DWORD count, LPHANDLE handles, DWORD timeout, LPDWORD result return 0; } if (timeout != INFINITE) { - ticks = GetTickCount(); - } + GetSystemTimeAsFileTime(&ticks.ft_val); + ticks.ft_i64 /= 10000; + } if (result == WAIT_OBJECT_0 + count) { /* Message has arrived - check it */ (void)win32_async_check(aTHX); @@ -2244,10 +2264,13 @@ win32_msgwait(pTHX_ DWORD count, LPHANDLE handles, DWORD timeout, LPDWORD result break; } } - /* compute time left to wait */ - ticks = timeout - ticks; /* If we are past the end say zero */ - return (ticks > 0) ? ticks : 0; + if(!ticks.ft_i64 || ticks.ft_i64 > endtime) + return 0; + /* compute time left to wait */ + ticks.ft_i64 = endtime - ticks.ft_i64; + /*if more ms than DWORD, then return max DWORD*/ + return ticks.ft_i64 <= UINT_MAX ? (DWORD)ticks.ft_i64:UINT_MAX; } int -- 1.7.9.msysgit.0 ```
p5pRT commented 12 years ago

From @steve-m-hay

bulk 88 via RT wrote on 2012-09-18​:

Sorry\, haven't found time yet to read this thread\, but alarm() emulation on Windows is supposed to interrupt sleep(). A quick check confirms that it still works​:

C​:\>perl -E"$SIG{ALRM}=sub{die};say time;alarm 2;eval{sleep 10};say time" 1347909521 1347909523

sleep() is also emulated\, so doesn't count as a "blocking system call". Maybe this should be documented in perlport.pod.

Cheers\, -Jant

But sleep was interrupted by the signal\, the sleep when *poof* with the longjmp/die. The was only possible with eval and die\, if you remove them and want Perl sleep to return early posix style on win32 it wont work.

_____________________________________ C​:\p517\perl>perl -E "$SIG{ALRM}=sub{0;};say time;alarm 2;sleep 10;say time" 1347955780 1347955790

C​:\p517\perl>r __________________________________

I don't think Win32 is alone in that. This is as documented in perlfunc/alarm​:

If you want to use C\ to time out a system call you need to use an C\/C\ pair. You can't rely on the alarm causing the system call to fail with C\<$!> set to C\ because Perl sets up signal handlers to restart system calls on some systems. Using C\/C\ always works\, modulo the caveats given in L\<perlipc/"Signals">.

  eval {   local $SIG{ALRM} = sub { die "alarm\n" }; # NB​: \n required   alarm $timeout;   $nread = sysread SOCKET\, $buffer\, $size;   alarm 0;   };   if ($@​) {   die unless $@​ eq "alarm\n"; # propagate unexpected errors   # timed out   }   else {   # didn't   }

I also attached the new patch to this post.

Thanks\, I will take a look later.

p5pRT commented 12 years ago

From @steve-m-hay

On Tue Sep 18 01​:44​:54 2012\, Steve.Hay@​verosoftware.com wrote​:

bulk 88 via RT wrote on 2012-09-18​:

I also attached the new patch to this post.

Thanks\, I will take a look later.

Thanks\, applied as 001e9f8966.

I removed the perlport bit about sleep not being interruptible by alarm in the light of my comment about that being true more generally than just Win32. It might be worth adding a note about sleep being emulated\, though\, as Jan suggested.

Also\, were you going to add a warning (like gmtime has) in the case of sleep's argument being too large? If not then I'll look at it myself - I think it would be worthwhile.

p5pRT commented 12 years ago

From @bulk88

On Tue Sep 18 09​:49​:03 2012\, shay wrote​:

Also\, were you going to add a warning (like gmtime has) in the case of sleep's argument being too large? If not then I'll look at it myself - I think it would be worthwhile.

I decided not to since I've never used the warnings category system in C and I couldn't decide whether the category should be "portable" on all Perls\, or "overflow" only on Win32 Perl\, or always on warning. I suggest you put it in. Silent failure or silent bad behavior (sleep sleeping orders of magnitude less time than requested) isn't good\, not everyone reads the manual\, and some don't even use warnings\, but those that use warnings will be saved from failure.

Thanks for applying the patch.

p5pRT commented 12 years ago

From @jkeenan

On Tue Sep 18 11​:03​:19 2012\, bulk88 wrote​:

On Tue Sep 18 09​:49​:03 2012\, shay wrote​:

Also\, were you going to add a warning (like gmtime has) in the case of sleep's argument being too large? If not then I'll look at it myself - I think it would be worthwhile.

I decided not to since I've never used the warnings category system in C and I couldn't decide whether the category should be "portable" on all Perls\, or "overflow" only on Win32 Perl\, or always on warning. I suggest you put it in. Silent failure or silent bad behavior (sleep sleeping orders of magnitude less time than requested) isn't good\, not everyone reads the manual\, and some don't even use warnings\, but those that use warnings will be saved from failure.

Thanks for applying the patch.

A patch has been applied. Can we close this ticket which (hint!) was first opened in 2004?

Thank you very much. Jim Keenan

p5pRT commented 12 years ago

From @steve-m-hay

The patch applied fixes the main bug here. The issues with VS debugger can be raised as a separate bug if necessary. I will apply a change to add a warning about sleep's argument being restricted on Windows and report here when I have done so.

p5pRT commented 12 years ago

From [Unknown Contact. See original ticket]

The patch applied fixes the main bug here. The issues with VS debugger can be raised as a separate bug if necessary. I will apply a change to add a warning about sleep's argument being restricted on Windows and report here when I have done so.

p5pRT commented 12 years ago

@steve-m-hay - Status changed from 'open' to 'resolved'

p5pRT commented 12 years ago

From @steve-m-hay

On Wed Sep 19 01​:06​:35 2012\, shay wrote​:

The patch applied fixes the main bug here. The issues with VS debugger can be raised as a separate bug if necessary. I will apply a change to add a warning about sleep's argument being restricted on Windows and report here when I have done so.

Warning now added in commit 3b9aea04d2. I also added the note about sleep being emulated on Windows in commit 3cd5044784.