Skip Menu |
Report information
Id: 121230
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: bulk88 <bulk88 [at] hotmail.com>
Cc:
AdminCc:

Operating System: mswin32
PatchStatus: (no value)
Severity: High
Type: core
Perl Version: 5.19.8
Fixed In: (no value)



From: bulk88 <bulk88 [...] hotmail.com>
Subject: process group kill on Win32 broken in 5.17.2, regression 5.18
Date: Tue, 11 Feb 2014 13:50:31 -0500
To: perlbug [...] perl.org, Father Chrysostomos <sprout [...] cpan.org>
Download (untitled) / with headers
text/plain 8.3k
This is a bug report for perl from bulk88@hotmail.com, generated with the help of perlbug 1.39 running under perl 5.19.8. ----------------------------------------------------------------- [Please describe your issue here] Patch http://perl5.git.perl.org/perl.git/c2fd40cb025bad682ee44b1ccb8ea501ed51924d from 5.17.2 and https://rt.perl.org/Ticket/Display.html?id=112990 broke the process group (actually process tree, since process groups don't exist on Win32, I'll call it a process group for the remainder of the ticket, -signal kills a process tree on Win32 Perl, not a process group) kill feature of Win32 Perl's kill() PP func. I attached a sample script of the bug, YMMV on what it does on POSIX. IDK and IDC what the script does on POSIX. If the script can't kill the child proc, you will need to manually kill the proc with Task Manager. The script returns success on 5.12, 5.14, 5.16, but fails on 5.18 and on blead. The fix for this will need to go into 5.18 maint if 5.18 maint is still open by the time there is a patch. ---------------------------------- DllExport int win32_kill(int pid, int sig) { ---------------------------------- On 5.16 and older, when running the test script, sig is -, and pid +. That patch changed it so sig is + and pid is -. A -pid on Win32 Perl is a psuedofork fake-PID. So on post-patch, entry win32_kill checks if PID is neg, takes the fake-PID branch ( http://perl5.git.perl.org/perl.git/blob/d048ecb4d187ebbafce763f1e0437120c6726967:/win32/win32.c#1372 ) looks up the bad -PID in the fake-PID table, doesn't find it, and returns with does ---------------------------------- errno = EINVAL; return -1; } ---------------------------------- Callstacks from the 2nd system proc in test script. ---------------------------------- Show quoted text
> perl516.dll!win32_kill(int pid=2288, int sig=-9) Line 1277 C
perl516.dll!PerlProcKillpg(IPerlProc * piPerl=0x00424700, int pid=2288, int sig=9) Line 1607 + 0xf bytes C++ perl516.dll!Perl_apply(interpreter * my_perl=0x01b94004, long type=303, sv * * mark=0x01b99934, sv * * sp=0x01b99934) Line 1746 + 0x20 bytes C perl516.dll!Perl_pp_chown(interpreter * my_perl=0x01b94004) Line 3558 + 0x24 bytes C perl516.dll!Perl_runops_debug(interpreter * my_perl=0x01b94004) Line 2119 + 0xf bytes C perl516.dll!S_run_body(interpreter * my_perl=0x01b94004, long oldscope=1) Line 2402 + 0xf bytes C perl516.dll!perl_run(interpreter * my_perl=0x01b94004) Line 2320 + 0xd bytes C perl516.dll!RunPerl(int argc=3, char * * argv=0x004231f0, char * * env=0x00426448) Line 270 + 0x9 bytes C++ perl.exe!main(int argc=3, char * * argv=0x004231f0, char * * env=0x00423600) Line 23 + 0x12 bytes C perl.exe!__tmainCRTStartup() Line 582 + 0x17 bytes C kernel32.dll!_BaseProcessStart@4() + 0x28 bytes --------------------------------- Show quoted text
> perl519.dll!win32_kill(int pid=-3464, int sig=9) Line 1369 C
perl519.dll!PerlProcKill(IPerlProc * piPerl=0x01b67950, int pid=-3464, int sig=9) Line 1599 + 0xd bytes C++ perl519.dll!Perl_apply(interpreter * my_perl=0x01b6858c, long type=304, sv * * mark=0x01b6cfdc, sv * * sp=0x01b6cfdc) Line 1797 + 0x20 bytes C perl519.dll!Perl_pp_chown(interpreter * my_perl=0x01b6858c) Line 3509 + 0x24 bytes C perl519.dll!Perl_runops_standard(interpreter * my_perl=0x01b6858c) Line 42 + 0xc bytes C perl519.dll!S_run_body(interpreter * my_perl=0x01b6858c, long oldscope=1) Line 2446 + 0xf bytes C perl519.dll!perl_run(interpreter * my_perl=0x01b6858c) Line 2365 C perl519.dll!RunPerl(int argc=3, char * * argv=0x01b631d0, char * * env=0x01b63248) Line 270 + 0x9 bytes C++ perl.exe!__tmainCRTStartup() Line 582 + 0x17 bytes C kernel32.dll!_BaseProcessStart@4() + 0x28 bytes --------------------------------- Another problem is that this removal in the patch is probably unacceptable on Win32 Perl. ------------------------------------------------ - APPLY_TAINT_PROPER(); -#ifdef HAS_KILLPG - if (PerlProc_killpg(proc,val)) /* BSD */ -#else - if (PerlProc_kill(-proc,val)) /* SYSV */ -#endif - tot--; ------------------------------------------------ Win32 Perl defines PerlProcKillpg as this ------------------------------------------------ int PerlProcKillpg(struct IPerlProc* piPerl, int pid, int sig) { return win32_kill(pid, -sig); } ------------------------------------------------- About the test script included, both system()ed perls are launched and hang off cmd.exe (win32 shell) processes. Why they are launched in shells is a separate issue for another ticket. How this regression was found? This report came from trying to eliminate harness from hanging/waiting against a child .t proc's watchdog()'s FULL time. Since system() returned the shell's PID, not the watchdog perl proc's PID, killing the shell didn't kill the watchdog process, and harness hung/waited 300 seconds in some cases (or whatever the watchdog is set for, and it is 300 seconds in some places like re/susbt.t) for the watchdog to timeout and naturally exit. There are a number of bugs and solutions in the watchdog problem, one attempt I tried was to kill the process tree (the cmd.exe and the watchdog perl proc underneath it) instead of just the shell. That didn't work in blead but it did from a old system perl, which lead to this ticket. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=high --- Site configuration information for perl 5.19.8: Configured by Owner at Thu Dec 26 04:12:47 2013. Summary of my perl5 (revision 5 version 19 subversion 8) configuration: Derived from: Platform: osname=MSWin32, osvers=5.1, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=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 -O1 -MD -Zi -DNDEBUG -G7 -GL -DWIN32 -D_CONSOLE -DNO_STRICT -DPERL_TEXTMODE_SCRIPTS -DPERL_HASH_FUNC_ONE_AT_A_TIME -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T', optimize='-O1 -MD -Zi -DNDEBUG -G7 -GL', cppflags='-DWIN32' ccversion='13.10.6030', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8 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 -ltcg -libpath:"c:\perl519\lib\CORE" -machine:x86' libpth="C:\Program Files\Microsoft Visual Studio .NET 2003\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 ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.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 ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl519.lib gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -ltcg -libpath:"c:\perl519\lib\CORE" -machine:x86' Locally applied patches: uncommitted-changes --- @INC for perl 5.19.8: C:/perl519/site/lib C:/perl519/lib . --- Environment for perl 5.19.8: HOME (unset) LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=C:\perl519\bin;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\IDE;C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\BIN;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools\bin\prerelease;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem; PERL_BADLANG (unset) SHELL (unset)
Download kill.pl
text/x-perl 134b
$pid = system(1, $^X.' -e "sleep 500"'); system(1, $^X.' -e "print kill(-9, '.$pid.') ? \"kill sucessful\n\" : \"couldnt kill\n\";');
Download killtestrun.txt
text/plain 6.2k
C:\>perl -V Summary of my perl5 (revision 5 version 19 subversion 4) configuration: Derived from: Platform: osname=MSWin32, osvers=5.2, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=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 -Od -MD -Zi -DNDEBUG -GS- -DWIN32 -D_CONS OLE -DNO_STRICT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DPERL_T EXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO', optimize='-Od -MD -Zi -DNDEBUG -GS-', cppflags='-DWIN32' ccversion='15.00.30729.01', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksi ze=8 alignbytes=8, prototype=define Linker and Libraries: ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -libpath:"c: \p519\lib\CORE" -machine:x86 "/manifestdependency:type='Win32' name='Microsoft. Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyTo ken='6595b64144ccf1df' language='*'"' libpth=\lib libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.l ib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_32 .lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt. lib perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg 32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws 2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msv crt.lib libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl519.lib gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf - libpath:"c:\p519\lib\CORE" -machine:x86 "/manifestdependency:type='Win32' name= 'Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'"' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES HAVE_INTERP_INTERN MULTIPLICITY PERLIO_LAYERS PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_IMPLICIT_CONTEXT PERL_IMPLICIT_SYS PERL_MALLOC_WRAP PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV USE_ITHREADS USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF Locally applied patches: uncommitted-changes Built under MSWin32 Compiled at Oct 3 2013 15:18:14 @INC: C:/p519/site/lib C:/p519/lib . C:\>perl "C:\Documents and Settings\Administrator\Desktop\kill.pl" C:\>couldnt kill C:\>p3216 C:\>set PATH=*REMOVED* C:\>set PERL_JSON_BACKEND=JSON::XS C:\>set PERL_YAML_BACKEND=YAML C:\>perl -V Summary of my perl5 (revision 5 version 16 subversion 3) configuration: Platform: osname=MSWin32, osvers=5.2, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=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 -DPERL_TEXTMODE_SCRIPTS -DUSE_SITECUSTOMIZE -DPERL_IMPLICIT_CONTEXT - DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T', optimize='-MD -Zi -DNDEBUG -O1', cppflags='-DWIN32' ccversion='15.0.30729', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksi ze=8 alignbytes=8, prototype=define Linker and Libraries: ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -libpath:"C: \Perl3216\lib\CORE" -machine:x86' libpth=\lib libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.l ib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_32 .lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt. lib perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg 32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws 2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msv crt.lib libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl516.lib gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf - libpath:"C:\Perl3216\lib\CORE" -machine:x86' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES HAVE_INTERP_INTERN MULTIPLICITY PERLIO_LAYERS PERL_DONT_CREATE_GVSV PERL_IMPLICIT_CONTEXT PERL_IMPLICIT_SYS PERL_MALLOC_WRAP PERL_PRESERVE_IVUV PL_OP_SLAB_ALLOC USE_ITHREADS USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF USE_SITECUSTOMIZE Locally applied patches: ActivePerl Build 1603 [296746] Built under MSWin32 Compiled at Mar 13 2013 11:29:21 %ENV: PERL_JSON_BACKEND="JSON::XS" PERL_YAML_BACKEND="YAML" @INC: C:/Perl3216/site/lib C:/Perl3216/lib . C:\>perl "C:\Documents and Settings\Administrator\Desktop\kill.pl" C:\>kill sucessful C:\>
From: Steve Hay <steve.m.hay [...] googlemail.com>
Date: Wed, 12 Feb 2014 09:06:45 +0000
To: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
CC: "bugs-bitbucket [...] rt.perl.org" <bugs-bitbucket [...] rt.perl.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
On 11 February 2014 18:52, bulk88 <perlbug-followup@perl.org> wrote: Show quoted text
> # New Ticket Created by bulk88 > # Please include the string: [perl #121230] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=121230 > > > > This is a bug report for perl from bulk88@hotmail.com, > generated with the help of perlbug 1.39 running under perl 5.19.8. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > Patch > http://perl5.git.perl.org/perl.git/c2fd40cb025bad682ee44b1ccb8ea501ed51924d > from 5.17.2 and https://rt.perl.org/Ticket/Display.html?id=112990 broke > the process group (actually process tree, since process groups don't > exist on Win32, I'll call it a process group for the remainder of the > ticket, -signal kills a process tree on Win32 Perl, not a process group) > kill feature of Win32 Perl's kill() PP func.
[...] Show quoted text
> > How this regression was found? This report came from trying to eliminate > harness from hanging/waiting against a child .t proc's watchdog()'s FULL > time. Since system() returned the shell's PID, not the watchdog perl > proc's PID, killing the shell didn't kill the watchdog process, and > harness hung/waited 300 seconds in some cases (or whatever the watchdog > is set for, and it is 300 seconds in some places like re/susbt.t) for > the watchdog to timeout and naturally exit. There are a number of bugs > and solutions in the watchdog problem, one attempt I tried was to kill > the process tree (the cmd.exe and the watchdog perl proc underneath it) > instead of just the shell. That didn't work in blead but it did from a > old system perl, which lead to this ticket. >
Thank you for finding this! It's bugged me for ages that re/subst_amp.t and re/subst_wamp.t both hang for ages when running the test suite, but I've never got round to looking into why :-/ Clearly I should have shouted because this is a nasty regression. This one really needs fixing. Should it even be a blocker for 5.20?
RT-Send-CC: perl5-porters [...] perl.org
On Wed Feb 12 01:07:10 2014, shay wrote: Show quoted text
> > Thank you for finding this! It's bugged me for ages that > re/subst_amp.t and re/subst_wamp.t both hang for ages when running the > test suite, but I've never got round to looking into why :-/
I will be very clear, this ticket isn't about that watchdog hang. Fixing kill() process group will not unhang that test. Show quoted text
> > Clearly I should have shouted because this is a nasty regression. > > This one really needs fixing. Should it even be a blocker for 5.20?
Yes because the feature worked in 5.12 to 5.16. IDK if it existed in < 5.12. I can't fix this because it is a POSIX, and I IDK enough in that area. I do not understand the complaint in #112990 and solution in there. Therefore my opinion is to git revert that commit, but that probably is not the right solution. If I do try to code it, I will probably break non-Win32 builds. Therefore I didn't offer a patch and I only supplied a test script and the details in the OP. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 198b
Bump. If there aren't any ideas from the posix people to fix this, the only option I see is to create a patch to revert c2fd40cb025bad682ee44b1ccb8ea501ed51924d . -- bulk88 ~ bulk88 at hotmail.com
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Date: Sun, 23 Feb 2014 13:57:23 +0000
To: bulk88 via RT <perlbug-followup [...] perl.org>
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 570b
On Sat, Feb 22, 2014 at 01:50:12AM -0800, bulk88 via RT wrote: Show quoted text
> Bump. If there aren't any ideas from the posix people to fix this, the only option I see is to create a patch to revert c2fd40cb025bad682ee44b1ccb8ea501ed51924d .
I've pushed a fix as smoke-me/davem/killpg. I haven't of course tested it on Win32. If you're able to confirm it works, and if the smokes are ok, I'll merge it into blead. It should then be a candidate for maint-5.18. Are you in a position to contribute Win32-specific tests for this? -- Standards (n). Battle insignia or tribal totems.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 504b
On Sun Feb 23 05:57:55 2014, davem wrote: Show quoted text
> > I've pushed a fix as smoke-me/davem/killpg. I haven't of course tested > it > on Win32. If you're able to confirm it works, and if the smokes are > ok, > I'll merge it into blead. It should then be a candidate for maint- > 5.18. > > Are you in a position to contribute Win32-specific tests for this?
Yes. It will be a day or 2 for me to try it. I'm not sure whether to add the test to t/op/kill0.t or t/win32/system.t. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
On Mon Feb 24 19:23:45 2014, bulk88 wrote: While writing a test for this, I discovered another bug or issue in Win32 PG kill. PG kill is supposed to return the number of processes "signaled" (is this a synonym for "killed" on Win32 platform?) per perldoc ------------------------------------------------ Sends a signal to a list of processes. Returns the number of processes successfully signaled (which is not necessarily the same as the number actually killed). ----------------------------------------------- My testing shows the retval in PP of PG kill in 5.12.2 is always 1. from win32_kill() ------------------------------------------- if (child >= 0) { if (my_kill(pid, sig)) {<<<<<<<<<<<<kill count lost DWORD exitcode = 0; if (GetExitCodeProcess(w32_child_handles[child], &exitcode) && exitcode != STILL_ACTIVE) { remove_dead_process(child); } return 0; } } -------------------------------------------- Perl_apply -------------------------------------------- if (val < 0) { val = -val; while (++mark <= sp) { I32 proc; if (!(SvIOK(*mark) || SvNOK(*mark) || looks_like_number(*mark))) Perl_croak(aTHX_ "Can't kill a non-numeric process ID"); proc = SvIV(*mark); APPLY_TAINT_PROPER(); #ifdef HAS_KILLPG if (PerlProc_killpg(proc,val)) /* BSD */ <<<<<<<<<<<<<no way to return kill count, but posix killpg doesn't return process count, just error code #else if (PerlProc_kill(-proc,val)) /* SYSV */ #endif -------------------------- Is this something to be fixed or it is correct behavior that you can't tell how many processes were in the group from a kill on a PG? -- bulk88 ~ bulk88 at hotmail.com
Date: Wed, 26 Feb 2014 14:03:45 +0000
To: bulk88 via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Tue, Feb 25, 2014 at 05:10:33PM -0800, bulk88 via RT wrote: Show quoted text
> While writing a test for this, I discovered another bug or issue in > Win32 PG kill. PG kill is supposed to return the number of processes > "signaled" (is this a synonym for "killed" on Win32 platform?) per > perldoc > ------------------------------------------------ > Sends a signal to a list of processes. Returns the number of processes > successfully signaled (which is not necessarily the same as the number > actually killed). > ----------------------------------------------- > > My testing shows the retval in PP of PG kill in 5.12.2 is always 1.
I think the perl docs are misleading. It's not supposed to indicate the total *processes* killed - UNIX certainly doesn't provide this info when killing groups - but rather the number of 'kills' in the list that were successful. i.e. kill $SIG, $proc1, $proc2, $no_such_process; returns 2. -- "But Sidley Park is already a picture, and a most amiable picture too. The slopes are green and gentle. The trees are companionably grouped at intervals that show them to advantage. The rill is a serpentine ribbon unwound from the lake peaceably contained by meadows on which the right amount of sheep are tastefully arranged." -- Lady Croom, "Arcadia"
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 810b
On Sun Feb 23 05:57:55 2014, davem wrote: Show quoted text
> On Sat, Feb 22, 2014 at 01:50:12AM -0800, bulk88 via RT wrote:
> > Bump. If there aren't any ideas from the posix people to fix this, > > the only option I see is to create a patch to revert > > c2fd40cb025bad682ee44b1ccb8ea501ed51924d .
> > I've pushed a fix as smoke-me/davem/killpg. I haven't of course tested > it > on Win32. If you're able to confirm it works, and if the smokes are > ok, > I'll merge it into blead. It should then be a candidate for maint- > 5.18. > > Are you in a position to contribute Win32-specific tests for this?
Test patch attached. Not fully smoked on Win32, but "perl harness op/kill0.t" passes. I'm not sure if the unix file perms on the new file in the patch are correct since my git is Win32. -- bulk88 ~ bulk88 at hotmail.com
Subject: 0001-RT-121230-create-tests-for-process-group-kill-on-Win.patch
From 7062d2991e08b2b516db88450d4337d83910d462 Mon Sep 17 00:00:00 2001 From: bulk88 <bulk88@hotmail.com> Date: Wed, 26 Feb 2014 17:31:18 -0500 Subject: [PATCH] RT #121230, create tests for process group kill on Win32 --- t/op/kill0.t | 41 ++++++++++++++++++++++++++++++++++++++++- t/op/kill0_child | 9 +++++++++ 2 files changed, 49 insertions(+), 1 deletions(-) create mode 100644 t/op/kill0_child diff --git a/t/op/kill0.t b/t/op/kill0.t index d3ef8f7..4012761 100644 --- a/t/op/kill0.t +++ b/t/op/kill0.t @@ -13,8 +13,9 @@ BEGIN { } use strict; +use Config; -plan tests => 6; +plan tests => 9; ok( kill(0, $$), 'kill(0, $pid) returns true if $pid exists' ); @@ -50,3 +51,41 @@ for my $case ( @bad_pids ) { $x =~ /(\d+)/; ok(eval { kill 0, $1 }, "can kill a number string in a magic variable"); } + +SKIP: { + skip 'custom process group kill() only on Win32', 3 if ($^O ne 'MSWin32'); + #create 2 child processes, an outer one created by kill0.t, and an inner one + #created by outer this allows the test to fail if only the outer one was + #killed, since the inner will stay around and eventually print failed and + #out of sequence TAP to harness + unlink('killchildstarted'); + die q|can't unlink| if -e 'killchildstarted'; + eval q|END{unlink('killchildstarted');}|; + my $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); + die 'PID is 0' if !$pid; + while( ! -e 'killchildstarted') { + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here + } + #ways to break this test manually, change '-KILL' to 'KILL', change $pid to a + #bogus number + is(kill('-KILL', $pid), 1, 'process group kill, named signal'); + + my ($i, %signo, @signame, $sig_name) = 0; + ($sig_name = $Config{sig_name}) || die "No signals?"; + foreach my $name (split(' ', $sig_name)) { + $signo{$name} = $i; + $signame[$i] = $name; + $i++; + } + ok(scalar keys %signo > 1 && exists $signo{KILL}, '$Config{sig_name} parsed correctly'); + die q|A child proc wasn't killed and did cleanup on its own| if ! -e 'killchildstarted'; + unlink('killchildstarted'); + die q|can't unlink| if -e 'killchildstarted'; + #no END block, done earlier + $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); + die 'PID is 0' if !$pid; + while( ! -e 'killchildstarted') { + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here + } + is(kill(-$signo{KILL}, $pid), 1, 'process group kill, numeric signal'); +} diff --git a/t/op/kill0_child b/t/op/kill0_child new file mode 100644 index 0000000..c3d5eb2 --- /dev/null +++ b/t/op/kill0_child @@ -0,0 +1,9 @@ +#$ARGV[0] is filename used to notify parent .t perl proc that all PIDs are +#started in the process tree +#number 9999/9998 is eye catching +system(1, $^X, '-e', 'sleep 5; print qq|not ok 9999 - inner child process wasn\'t killed\n|;'); +system('echo outer child started > "'.$ARGV[0].'"'); +sleep 5; +#execution won't be reached if test successful +print "not ok 9998 - outer child process wasn\'t killed\n"; +unlink($ARGV[0]); -- 1.7.9.msysgit.0
RT-Send-CC: perl5-porters [...] perl.org
On Wed Feb 26 06:04:15 2014, davem wrote: Show quoted text
> On Tue, Feb 25, 2014 at 05:10:33PM -0800, bulk88 via RT wrote:
> > While writing a test for this, I discovered another bug or issue in > > Win32 PG kill. PG kill is supposed to return the number of processes > > "signaled" (is this a synonym for "killed" on Win32 platform?) per > > perldoc > > ------------------------------------------------ > > Sends a signal to a list of processes. Returns the number of processes > > successfully signaled (which is not necessarily the same as the number > > actually killed). > > ----------------------------------------------- > > > > My testing shows the retval in PP of PG kill in 5.12.2 is always 1.
> > I think the perl docs are misleading. It's not supposed to indicate > the total *processes* killed - UNIX certainly doesn't provide this info > when killing groups - but rather the number of 'kills' in the list > that were successful. i.e. > > kill $SIG, $proc1, $proc2, $no_such_process; > > returns 2. >
Patch for POD. -- bulk88 ~ bulk88 at hotmail.com
Subject: 0001-clarify-kill-s-POD.patch
From 8a0503965f4d6e9165156115692d8f7a8338495c Mon Sep 17 00:00:00 2001 From: bulk88 <bulk88@hotmail.com> Date: Wed, 26 Feb 2014 18:14:24 -0500 Subject: [PATCH] clarify kill's POD part of RT #121230 --- pod/perlfunc.pod | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod index 14c5171..8a3aaab 100644 --- a/pod/perlfunc.pod +++ b/pod/perlfunc.pod @@ -3218,7 +3218,7 @@ X<kill> X<signal> Sends a signal to a list of processes. Returns the number of processes successfully signaled (which is not necessarily the -same as the number actually killed). +same as the number actually killed) from the list of processes supplied. $cnt = kill 'HUP', $child1, $child2; kill 'KILL', @goners; -- 1.7.9.msysgit.0
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 502b
On Wed Feb 26 14:40:54 2014, bulk88 wrote: Show quoted text
> Test patch attached. Not fully smoked on Win32, but "perl harness > op/kill0.t" passes. I'm not sure if the unix file perms on the new > file in the patch are correct since my git is Win32.
This patch is broken. Fails manifest.t and I forgot perldelta.pod. I have manifest.t fixed, but I am having a perldelta.pod and podcheck.t problem that is being addressed in a separate ML thread. So wait until I have a new patch. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 340b
On Wed Feb 26 23:06:03 2014, bulk88 wrote: Show quoted text
> > This patch is broken. Fails manifest.t and I forgot perldelta.pod. I > have manifest.t fixed, but I am having a perldelta.pod and podcheck.t > problem that is being addressed in a separate ML thread. So wait until > I have a new patch.
New patch attached. -- bulk88 ~ bulk88 at hotmail.com
Subject: 0001-RT-121230-create-tests-for-process-group-kill-on-Win.patch
From dc146462729d13b3038d9a58ac6291bc2e09f77f Mon Sep 17 00:00:00 2001 From: bulk88 <bulk88@hotmail.com> Date: Thu, 27 Feb 2014 04:00:43 -0500 Subject: [PATCH] RT #121230, create tests for process group kill on Win32 --- MANIFEST | 3 ++- pod/perldelta.pod | 9 +++++++-- t/op/kill0.t | 41 ++++++++++++++++++++++++++++++++++++++++- t/op/kill0_child | 9 +++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 t/op/kill0_child diff --git a/MANIFEST b/MANIFEST index 9fcb518..e7ab59a 100644 --- a/MANIFEST +++ b/MANIFEST @@ -5238,7 +5238,8 @@ t/op/index.t See if index works t/op/index_thr.t See if index works in another thread t/op/int.t See if int works t/op/join.t See if join works -t/op/kill0.t See if kill(0, $pid) works +t/op/kill0_child Process tree script that is kill()ed +t/op/kill0.t See if kill works t/op/kvaslice.t See if index/value array slices work t/op/kvhslice.t See if key/value hash slices work t/op/lc.t See if lc, uc, lcfirst, ucfirst, quotemeta work diff --git a/pod/perldelta.pod b/pod/perldelta.pod index ab26873..f298d47 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -319,11 +319,16 @@ and compilation changes or changes in portability/compatibility. However, changes within modules for platforms should generally be listed in the L</Modules and Pragmata> section. +=head3 Win32 + =over 4 -=item XXX-some-platform +=item * -XXX +Killing a process tree with L<perlfunc/kill> and a negative signal, was broken +starting in 5.17.2. In this bug, C<kill> always returned 0 for a negative +signal even for valid PIDs, and no processes were terminated. This has been +fixed [perl #121230]. =back diff --git a/t/op/kill0.t b/t/op/kill0.t index d3ef8f7..4012761 100644 --- a/t/op/kill0.t +++ b/t/op/kill0.t @@ -13,8 +13,9 @@ BEGIN { } use strict; +use Config; -plan tests => 6; +plan tests => 9; ok( kill(0, $$), 'kill(0, $pid) returns true if $pid exists' ); @@ -50,3 +51,41 @@ for my $case ( @bad_pids ) { $x =~ /(\d+)/; ok(eval { kill 0, $1 }, "can kill a number string in a magic variable"); } + +SKIP: { + skip 'custom process group kill() only on Win32', 3 if ($^O ne 'MSWin32'); + #create 2 child processes, an outer one created by kill0.t, and an inner one + #created by outer this allows the test to fail if only the outer one was + #killed, since the inner will stay around and eventually print failed and + #out of sequence TAP to harness + unlink('killchildstarted'); + die q|can't unlink| if -e 'killchildstarted'; + eval q|END{unlink('killchildstarted');}|; + my $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); + die 'PID is 0' if !$pid; + while( ! -e 'killchildstarted') { + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here + } + #ways to break this test manually, change '-KILL' to 'KILL', change $pid to a + #bogus number + is(kill('-KILL', $pid), 1, 'process group kill, named signal'); + + my ($i, %signo, @signame, $sig_name) = 0; + ($sig_name = $Config{sig_name}) || die "No signals?"; + foreach my $name (split(' ', $sig_name)) { + $signo{$name} = $i; + $signame[$i] = $name; + $i++; + } + ok(scalar keys %signo > 1 && exists $signo{KILL}, '$Config{sig_name} parsed correctly'); + die q|A child proc wasn't killed and did cleanup on its own| if ! -e 'killchildstarted'; + unlink('killchildstarted'); + die q|can't unlink| if -e 'killchildstarted'; + #no END block, done earlier + $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); + die 'PID is 0' if !$pid; + while( ! -e 'killchildstarted') { + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here + } + is(kill(-$signo{KILL}, $pid), 1, 'process group kill, numeric signal'); +} diff --git a/t/op/kill0_child b/t/op/kill0_child new file mode 100644 index 0000000..c3d5eb2 --- /dev/null +++ b/t/op/kill0_child @@ -0,0 +1,9 @@ +#$ARGV[0] is filename used to notify parent .t perl proc that all PIDs are +#started in the process tree +#number 9999/9998 is eye catching +system(1, $^X, '-e', 'sleep 5; print qq|not ok 9999 - inner child process wasn\'t killed\n|;'); +system('echo outer child started > "'.$ARGV[0].'"'); +sleep 5; +#execution won't be reached if test successful +print "not ok 9998 - outer child process wasn\'t killed\n"; +unlink($ARGV[0]); -- 1.7.9.msysgit.0
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
CC: Perl5 Porters Mailing List <perl5-porters [...] perl.org>
Date: Thu, 27 Feb 2014 12:01:02 +0100
To: Brian Fraser via RT <perlbug-followup [...] perl.org>
From: Brian Fraser <fraserbn [...] gmail.com>
Download (untitled) / with headers
text/plain 5.4k
On Thu, Feb 27, 2014 at 10:07 AM, bulk88 via RT <perlbug-followup@perl.org> wrote:
Show quoted text
On Wed Feb 26 23:06:03 2014, bulk88 wrote:
>
> This patch is broken. Fails manifest.t and I forgot perldelta.pod. I
> have manifest.t fixed, but I am having a perldelta.pod and podcheck.t
> problem that is being addressed in a separate ML thread. So wait until
> I have a new patch.

New patch attached.

--
bulk88 ~ bulk88 at hotmail.com

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

From dc146462729d13b3038d9a58ac6291bc2e09f77f Mon Sep 17 00:00:00 2001
From: bulk88 <bulk88@hotmail.com>
Date: Thu, 27 Feb 2014 04:00:43 -0500
Subject: [PATCH] RT #121230, create tests for process group kill on Win32

---
 MANIFEST          |    3 ++-
 pod/perldelta.pod |    9 +++++++--
 t/op/kill0.t      |   41 ++++++++++++++++++++++++++++++++++++++++-
 t/op/kill0_child  |    9 +++++++++
 4 files changed, 58 insertions(+), 4 deletions(-)
 create mode 100644 t/op/kill0_child

diff --git a/MANIFEST b/MANIFEST
index 9fcb518..e7ab59a 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -5238,7 +5238,8 @@ t/op/index.t                      See if index works
 t/op/index_thr.t               See if index works in another thread
 t/op/int.t                     See if int works
 t/op/join.t                    See if join works
-t/op/kill0.t                   See if kill(0, $pid) works
+t/op/kill0_child               Process tree script that is kill()ed
+t/op/kill0.t                   See if kill works
 t/op/kvaslice.t                        See if index/value array slices work
 t/op/kvhslice.t                        See if key/value hash slices work
 t/op/lc.t                      See if lc, uc, lcfirst, ucfirst, quotemeta work
diff --git a/pod/perldelta.pod b/pod/perldelta.pod
index ab26873..f298d47 100644
--- a/pod/perldelta.pod
+++ b/pod/perldelta.pod
@@ -319,11 +319,16 @@ and compilation changes or changes in portability/compatibility.  However,
 changes within modules for platforms should generally be listed in the
 L</Modules and Pragmata> section.

+=head3 Win32
+
 =over 4

-=item XXX-some-platform
+=item *

-XXX
+Killing a process tree with L<perlfunc/kill> and a negative signal, was broken
+starting in 5.17.2. In this bug, C<kill> always returned 0 for a negative
+signal even for valid PIDs, and no processes were terminated. This has been
+fixed [perl #121230].

Better to use 5.18.0 here -- the vast majority of users have no reason to care about dev releases.
 
Show quoted text

 =back

diff --git a/t/op/kill0.t b/t/op/kill0.t
index d3ef8f7..4012761 100644
--- a/t/op/kill0.t
+++ b/t/op/kill0.t
@@ -13,8 +13,9 @@ BEGIN {
 }

 use strict;
+use Config;

-plan tests => 6;
+plan tests => 9;

 ok( kill(0, $$), 'kill(0, $pid) returns true if $pid exists' );

@@ -50,3 +51,41 @@ for my $case ( @bad_pids ) {
   $x =~ /(\d+)/;
   ok(eval { kill 0, $1 }, "can kill a number string in a magic variable");
 }
+
+SKIP: {
+  skip 'custom process group kill() only on Win32', 3 if ($^O ne 'MSWin32');
+  #create 2 child processes, an outer one created by kill0.t, and an inner one
+  #created by outer this allows the test to fail if only the outer one was
+  #killed, since the inner will stay around and eventually print failed and
+  #out of sequence TAP to harness
+  unlink('killchildstarted');
+  die q|can't unlink| if -e 'killchildstarted';
+  eval q|END{unlink('killchildstarted');}|;
+  my $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted');
+  die 'PID is 0' if !$pid;
+  while( ! -e 'killchildstarted') {
+    sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here
+  }
+  #ways to break this test manually, change '-KILL' to 'KILL', change $pid to a
+  #bogus number
+  is(kill('-KILL', $pid), 1, 'process group kill, named signal');
+
+  my ($i, %signo, @signame, $sig_name) = 0;
+  ($sig_name = $Config{sig_name}) || die "No signals?";
+  foreach my $name (split(' ', $sig_name)) {
+    $signo{$name} = $i;
+    $signame[$i] = $name;
+    $i++;
+  }
+  ok(scalar keys %signo > 1 && exists $signo{KILL}, '$Config{sig_name} parsed correctly');
+  die q|A child proc wasn't killed and did cleanup on its own| if ! -e 'killchildstarted';
+  unlink('killchildstarted');
+  die q|can't unlink| if -e 'killchildstarted';
+  #no END block, done earlier
+  $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted');
+  die 'PID is 0' if !$pid;
+  while( ! -e 'killchildstarted') {
+    sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here
+  }
+  is(kill(-$signo{KILL}, $pid), 1, 'process group kill, numeric signal');
+}
diff --git a/t/op/kill0_child b/t/op/kill0_child
new file mode 100644
index 0000000..c3d5eb2
--- /dev/null
+++ b/t/op/kill0_child
@@ -0,0 +1,9 @@
+#$ARGV[0] is filename used to notify parent .t perl proc that all PIDs are
+#started in the process tree
+#number 9999/9998 is eye catching
+system(1, $^X, '-e', 'sleep 5; print qq|not ok 9999 - inner child process wasn\'t killed\n|;');
+system('echo outer child started > "'.$ARGV[0].'"');
+sleep 5;

^ how thoroughly was this tested? <10 second waits tend to fail on VMs quite often.
 
Show quoted text
+#execution won't be reached if test successful
+print "not ok 9998 - outer child process wasn\'t killed\n";
+unlink($ARGV[0]);
--
1.7.9.msysgit.0



RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 6.8k
On Thu Feb 27 03:01:28 2014, Hugmeir wrote: Show quoted text
> On Thu, Feb 27, 2014 at 10:07 AM, bulk88 via RT
> > New patch attached. > > > > -- > > bulk88 ~ bulk88 at hotmail.com > > > > --- > > via perlbug: queue: perl5 status: open > > https://rt.perl.org/Ticket/Display.html?id=121230 > > > > From dc146462729d13b3038d9a58ac6291bc2e09f77f Mon Sep 17 00:00:00 > > 2001 > > From: bulk88 <bulk88@hotmail.com> > > Date: Thu, 27 Feb 2014 04:00:43 -0500 > > Subject: [PATCH] RT #121230, create tests for process group kill on > > Win32 > > > > --- > > MANIFEST | 3 ++- > > pod/perldelta.pod | 9 +++++++-- > > t/op/kill0.t | 41 ++++++++++++++++++++++++++++++++++++++++- > > t/op/kill0_child | 9 +++++++++ > > 4 files changed, 58 insertions(+), 4 deletions(-) > > create mode 100644 t/op/kill0_child > > > > diff --git a/MANIFEST b/MANIFEST > > index 9fcb518..e7ab59a 100644 > > --- a/MANIFEST > > +++ b/MANIFEST > > @@ -5238,7 +5238,8 @@ t/op/index.t See if index > > works > > t/op/index_thr.t See if index works in another thread > > t/op/int.t See if int works > > t/op/join.t See if join works > > -t/op/kill0.t See if kill(0, $pid) works > > +t/op/kill0_child Process tree script that is kill()ed > > +t/op/kill0.t See if kill works > > t/op/kvaslice.t See if index/value array > > slices > > work > > t/op/kvhslice.t See if key/value hash slices > > work > > t/op/lc.t See if lc, uc, lcfirst, ucfirst, > > quotemeta > > work > > diff --git a/pod/perldelta.pod b/pod/perldelta.pod > > index ab26873..f298d47 100644 > > --- a/pod/perldelta.pod > > +++ b/pod/perldelta.pod > > @@ -319,11 +319,16 @@ and compilation changes or changes in > > portability/compatibility. However, > > changes within modules for platforms should generally be listed in > > the > > L</Modules and Pragmata> section. > > > > +=head3 Win32 > > + > > =over 4 > > > > -=item XXX-some-platform > > +=item * > > > > -XXX > > +Killing a process tree with L<perlfunc/kill> and a negative signal, > > was > > broken > > +starting in 5.17.2. In this bug, C<kill> always returned 0 for a > > negative > > +signal even for valid PIDs, and no processes were terminated. This > > has > > been > > +fixed [perl #121230]. > >
> > Better to use 5.18.0 here -- the vast majority of users have no reason > to > care about dev releases.
Will revise. But I need a response to my comments about the next comment first. Show quoted text
> >
> > > > =back > > > > diff --git a/t/op/kill0.t b/t/op/kill0.t > > index d3ef8f7..4012761 100644 > > --- a/t/op/kill0.t > > +++ b/t/op/kill0.t > > @@ -13,8 +13,9 @@ BEGIN { > > } > > > > use strict; > > +use Config; > > > > -plan tests => 6; > > +plan tests => 9; > > > > ok( kill(0, $$), 'kill(0, $pid) returns true if $pid exists' ); > > > > @@ -50,3 +51,41 @@ for my $case ( @bad_pids ) { > > $x =~ /(\d+)/; > > ok(eval { kill 0, $1 }, "can kill a number string in a magic > > variable"); > > } > > + > > +SKIP: { > > + skip 'custom process group kill() only on Win32', 3 if ($^O ne > > 'MSWin32'); > > + #create 2 child processes, an outer one created by kill0.t, and an > > inner one > > + #created by outer this allows the test to fail if only the outer > > one was > > + #killed, since the inner will stay around and eventually print > > failed > > and > > + #out of sequence TAP to harness > > + unlink('killchildstarted'); > > + die q|can't unlink| if -e 'killchildstarted'; > > + eval q|END{unlink('killchildstarted');}|; > > + my $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); > > + die 'PID is 0' if !$pid; > > + while( ! -e 'killchildstarted') { > > + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here > > + } > > + #ways to break this test manually, change '-KILL' to 'KILL', > > change > > $pid to a > > + #bogus number > > + is(kill('-KILL', $pid), 1, 'process group kill, named signal'); > > + > > + my ($i, %signo, @signame, $sig_name) = 0; > > + ($sig_name = $Config{sig_name}) || die "No signals?"; > > + foreach my $name (split(' ', $sig_name)) { > > + $signo{$name} = $i; > > + $signame[$i] = $name; > > + $i++; > > + } > > + ok(scalar keys %signo > 1 && exists $signo{KILL}, > > '$Config{sig_name} > > parsed correctly'); > > + die q|A child proc wasn't killed and did cleanup on its own| if ! > > -e > > 'killchildstarted'; > > + unlink('killchildstarted'); > > + die q|can't unlink| if -e 'killchildstarted'; > > + #no END block, done earlier > > + $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); > > + die 'PID is 0' if !$pid; > > + while( ! -e 'killchildstarted') { > > + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here > > + } > > + is(kill(-$signo{KILL}, $pid), 1, 'process group kill, numeric > > signal'); > > +} > > diff --git a/t/op/kill0_child b/t/op/kill0_child > > new file mode 100644 > > index 0000000..c3d5eb2 > > --- /dev/null > > +++ b/t/op/kill0_child > > @@ -0,0 +1,9 @@ > > +#$ARGV[0] is filename used to notify parent .t perl proc that all > > PIDs are > > +#started in the process tree > > +#number 9999/9998 is eye catching > > +system(1, $^X, '-e', 'sleep 5; print qq|not ok 9999 - inner child > > process > > wasn\'t killed\n|;'); > > +system('echo outer child started > "'.$ARGV[0].'"'); > > +sleep 5; > >
> > ^ how thoroughly was this tested? <10 second waits tend to fail on VMs > quite often. >
Ran it a couple dozen times repeatedly breaking it by giving bogus signals, + signals, and fake pids and made sure harness failed as a not ok fail or out of sequence fail. Originally kill0.t did a "sleep 1;" to wait for the outer and inner procs to start. You dont want to kill the outer before it starts the inner proc which is what I encountered if the kill was sent with no delay after the system(). I then realized this is begging for a race condition and subsequent test fail in a VM with rough host timeslicing. I then switched to using the disk file. Win32::Event isn't core http://search.cpan.org/~cjm/Win32-IPC-1.09/lib/Win32/Event.pm . So I can't use that as IPC. It also is XS. Sleeping a very high number near infinity sounds like a bad idea to me and a way to hang a smoker if something goes wrong (and what will go wrong can't be predicted). I picked 5 secs because it seems like a good balance between waiting the kill0.t to get back its timeslice and do the kill, vs I/O blocking harness since kill0.t failed to kill the 2 child procs (future or manual breakage), and now harness is waiting 4 seconds until the 2 child procs close their stdio handles, and probably (assuming a lack of random kernel memory corruption and being a couple ms away from a panic/bsod) will write "not ok" to the tap stream. Also I picked 5 instead of 3 secs because of a possibility of future parallel testing on Win32. -- bulk88 ~ bulk88 at hotmail.com
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Date: Fri, 28 Feb 2014 14:02:20 +0000
To: bulk88 via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 454b
On Sun, Feb 23, 2014 at 01:57:23PM +0000, Dave Mitchell wrote: Show quoted text
> I've pushed a fix as smoke-me/davem/killpg. I haven't of course tested it > on Win32. If you're able to confirm it works, and if the smokes are ok, > I'll merge it into blead. It should then be a candidate for maint-5.18.
Now in blead as 111f73b5d79a652d1b6c6e3df3c6cd4c14c17ea7 -- Indomitable in retreat, invincible in advance, insufferable in victory -- Churchill on Montgomery
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 454b
On Thu Feb 27 09:02:49 2014, bulk88 wrote: Show quoted text
> On Thu Feb 27 03:01:28 2014, Hugmeir wrote:
> > Better to use 5.18.0 here -- the vast majority of users have no > > reason > > to > > care about dev releases.
> > Will revise. But I need a response to my comments about the next > comment first.
I don't think I will get a response. So it stays at 5 seconds and a new patch mentioning 5.18.0 instead of 5.17.2 is attached. -- bulk88 ~ bulk88 at hotmail.com
Subject: 0001-RT-121230-create-tests-for-process-group-kill-on-Win.patch
From d37c11f7b2b728856f89ce7f8afc7665f6922d8a Mon Sep 17 00:00:00 2001 From: bulk88 <bulk88@hotmail.com> Date: Fri, 28 Feb 2014 23:56:54 -0500 Subject: [PATCH] RT #121230, create tests for process group kill on Win32 --- MANIFEST | 3 ++- pod/perldelta.pod | 9 +++++++-- t/op/kill0.t | 41 ++++++++++++++++++++++++++++++++++++++++- t/op/kill0_child | 9 +++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 t/op/kill0_child diff --git a/MANIFEST b/MANIFEST index 9fcb518..e7ab59a 100644 --- a/MANIFEST +++ b/MANIFEST @@ -5238,7 +5238,8 @@ t/op/index.t See if index works t/op/index_thr.t See if index works in another thread t/op/int.t See if int works t/op/join.t See if join works -t/op/kill0.t See if kill(0, $pid) works +t/op/kill0_child Process tree script that is kill()ed +t/op/kill0.t See if kill works t/op/kvaslice.t See if index/value array slices work t/op/kvhslice.t See if key/value hash slices work t/op/lc.t See if lc, uc, lcfirst, ucfirst, quotemeta work diff --git a/pod/perldelta.pod b/pod/perldelta.pod index ab26873..1f413ff 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -319,11 +319,16 @@ and compilation changes or changes in portability/compatibility. However, changes within modules for platforms should generally be listed in the L</Modules and Pragmata> section. +=head3 Win32 + =over 4 -=item XXX-some-platform +=item * -XXX +Killing a process tree with L<perlfunc/kill> and a negative signal, was broken +starting in 5.18.0. In this bug, C<kill> always returned 0 for a negative +signal even for valid PIDs, and no processes were terminated. This has been +fixed [perl #121230]. =back diff --git a/t/op/kill0.t b/t/op/kill0.t index d3ef8f7..4012761 100644 --- a/t/op/kill0.t +++ b/t/op/kill0.t @@ -13,8 +13,9 @@ BEGIN { } use strict; +use Config; -plan tests => 6; +plan tests => 9; ok( kill(0, $$), 'kill(0, $pid) returns true if $pid exists' ); @@ -50,3 +51,41 @@ for my $case ( @bad_pids ) { $x =~ /(\d+)/; ok(eval { kill 0, $1 }, "can kill a number string in a magic variable"); } + +SKIP: { + skip 'custom process group kill() only on Win32', 3 if ($^O ne 'MSWin32'); + #create 2 child processes, an outer one created by kill0.t, and an inner one + #created by outer this allows the test to fail if only the outer one was + #killed, since the inner will stay around and eventually print failed and + #out of sequence TAP to harness + unlink('killchildstarted'); + die q|can't unlink| if -e 'killchildstarted'; + eval q|END{unlink('killchildstarted');}|; + my $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); + die 'PID is 0' if !$pid; + while( ! -e 'killchildstarted') { + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here + } + #ways to break this test manually, change '-KILL' to 'KILL', change $pid to a + #bogus number + is(kill('-KILL', $pid), 1, 'process group kill, named signal'); + + my ($i, %signo, @signame, $sig_name) = 0; + ($sig_name = $Config{sig_name}) || die "No signals?"; + foreach my $name (split(' ', $sig_name)) { + $signo{$name} = $i; + $signame[$i] = $name; + $i++; + } + ok(scalar keys %signo > 1 && exists $signo{KILL}, '$Config{sig_name} parsed correctly'); + die q|A child proc wasn't killed and did cleanup on its own| if ! -e 'killchildstarted'; + unlink('killchildstarted'); + die q|can't unlink| if -e 'killchildstarted'; + #no END block, done earlier + $pid = system(1, $^X, 'op/kill0_child', 'killchildstarted'); + die 'PID is 0' if !$pid; + while( ! -e 'killchildstarted') { + sleep 1; #a sleep 0 with $i++ will takes ~160 iterations here + } + is(kill(-$signo{KILL}, $pid), 1, 'process group kill, numeric signal'); +} diff --git a/t/op/kill0_child b/t/op/kill0_child new file mode 100644 index 0000000..c3d5eb2 --- /dev/null +++ b/t/op/kill0_child @@ -0,0 +1,9 @@ +#$ARGV[0] is filename used to notify parent .t perl proc that all PIDs are +#started in the process tree +#number 9999/9998 is eye catching +system(1, $^X, '-e', 'sleep 5; print qq|not ok 9999 - inner child process wasn\'t killed\n|;'); +system('echo outer child started > "'.$ARGV[0].'"'); +sleep 5; +#execution won't be reached if test successful +print "not ok 9998 - outer child process wasn\'t killed\n"; +unlink($ARGV[0]); -- 1.7.9.msysgit.0
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 220b
On Fri Feb 28 20:59:15 2014, bulk88 wrote: Show quoted text
> > I don't think I will get a response. So it stays at 5 seconds and a > new patch mentioning 5.18.0 instead of 5.17.2 is attached.
Bump. -- bulk88 ~ bulk88 at hotmail.com
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
To: bulk88 via RT <perlbug-followup [...] perl.org>
Date: Mon, 17 Mar 2014 16:38:23 +0000
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Fri, Feb 28, 2014 at 08:59:15PM -0800, bulk88 via RT wrote: Show quoted text
> I don't think I will get a response. So it stays at 5 seconds and a new > patch mentioning 5.18.0 instead of 5.17.2 is attached.
Thanks, applied as af728ca1bc. I also took the libery of cleaning the code up a bit with the following commit: commit 4c0e595c606032a1e1c0a922399a1e7bd2358ca9 Author: David Mitchell <davem@iabyn.com> AuthorDate: Mon Mar 17 16:19:10 2014 +0000 Commit: David Mitchell <davem@iabyn.com> CommitDate: Mon Mar 17 16:35:07 2014 +0000 tidy up kill0.t and kill0_child The previous commit added some tests to kill0.t, and added the auxiliary file kill0_child. Tidy up the new code to better match normal standards. In particular, improve the format, grammar and clarity of the comments, and replace q|...| with "..." where appropriate. Also, make the temporary filename a variable, and prefix it with "tmp-", so that if gets left around for any reason, it's more obvious that it's just an extraneous temporary file. (I haven't actually tested this commit on win32) -- "Strange women lying in ponds distributing swords is no basis for a system of government. Supreme executive power derives from a mandate from the masses, not from some farcical aquatic ceremony." -- Dennis, "Monty Python and the Holy Grail"
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
To: bulk88 via RT <perlbug-followup [...] perl.org>
Date: Mon, 17 Mar 2014 16:51:02 +0000
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Download (untitled) / with headers
text/plain 1.2k
On Wed, Feb 26, 2014 at 03:16:25PM -0800, bulk88 via RT wrote: Show quoted text
> > I think the perl docs are misleading. It's not supposed to indicate > > the total *processes* killed - UNIX certainly doesn't provide this info > > when killing groups - but rather the number of 'kills' in the list > > that were successful. i.e. > > > > kill $SIG, $proc1, $proc2, $no_such_process; > > > > returns 2. > >
> > Patch for POD.
Show quoted text
> Sends a signal to a list of processes. Returns the number of > processes successfully signaled (which is not necessarily the > -same as the number actually killed). > +same as the number actually killed) from the list of processes supplied.
Thanks. However, I ended up rewording it again as follows, with 12733a03, since I think the process / process group distinction still wasn't clear enough. -Sends a signal to a list of processes. Returns the number of -processes successfully signaled (which is not necessarily the -same as the number actually killed). +Sends a signal to a list of processes. Returns the number of arguments +that were successfully used to signal (which is not necessarily the same +as the number of processes actually killed, e.g. where a process group is +killed). -- In England there is a special word which means the last sunshine of the summer. That word is "spring".
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Mon Mar 17 09:40:43 2014, davem wrote: Show quoted text
> Thanks, applied as af728ca1bc. I also took the libery of cleaning the code > up a bit with the following commit: > > commit 4c0e595c606032a1e1c0a922399a1e7bd2358ca9 > Author: David Mitchell <davem@iabyn.com> > AuthorDate: Mon Mar 17 16:19:10 2014 +0000 > Commit: David Mitchell <davem@iabyn.com> > CommitDate: Mon Mar 17 16:35:07 2014 +0000 > > tidy up kill0.t and kill0_child > > The previous commit added some tests to kill0.t, and added the auxiliary > file kill0_child. Tidy up the new code to better match normal standards. > In particular, improve the format, grammar and clarity of the comments, > and replace q|...| with "..." where appropriate. > Also, make the temporary filename a variable, and prefix it with "tmp-", > so that if gets left around for any reason, it's more obvious that it's > just an extraneous temporary file.
Everything is fine in that except 1 thing. Why replace the single quotes with double quotes for the sake of using double quotes? I've always assume double quotes take longer for the compiler to parse than single quotes, and using double quotes without knowing why leads to a larger change of unintended code execution/interpolation. -- bulk88 ~ bulk88 at hotmail.com
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
To: bulk88 via RT <perlbug-followup [...] perl.org>
Date: Tue, 18 Mar 2014 12:03:59 +0000
Download (untitled) / with headers
text/plain 1.3k
On Mon, Mar 17, 2014 at 02:43:56PM -0700, bulk88 via RT wrote: Show quoted text
> Everything is fine in that except 1 thing. > > Why replace the single quotes with double quotes for the sake of using > double quotes? I've always assume double quotes take longer for the > compiler to parse than single quotes, and using double quotes without > knowing why leads to a larger change of unintended code > execution/interpolation.
The difference in parsing time between single and double quotes strings is infinitesimal and should not be a consideration in a small test file, especially one that does a lot of forking and sleeping. So I went for readability. IMO, "that's all" is much easier for the brain to parse than q|that's all| since the '|' character in perl isn't normally associated with string delimiting. The choice of single verses double-quoted is debatable. It can cause errors either way. For example someone later modifying your version might accidentally add: q|that's all, $person| while someone modifying my code might do "that's all you get for $100" In either case, if the programmer doesn't check the quotedness of string before modifying it, they get everything they deserve. -- A power surge on the Bridge is rapidly and correctly diagnosed as a faulty capacitor by the highly-trained and competent engineering staff. -- Things That Never Happen in "Star Trek" #9
From: demerphq <demerphq [...] gmail.com>
Date: Tue, 18 Mar 2014 13:15:11 +0100
To: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
CC: bulk88 via RT <perlbug-followup [...] perl.org>, Perl5 Porteros <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 678b
On 18 March 2014 13:03, Dave Mitchell <davem@iabyn.com> wrote: Show quoted text
> On Mon, Mar 17, 2014 at 02:43:56PM -0700, bulk88 via RT wrote:
>> Everything is fine in that except 1 thing. >> >> Why replace the single quotes with double quotes for the sake of using >> double quotes? I've always assume double quotes take longer for the >> compiler to parse than single quotes, and using double quotes without >> knowing why leads to a larger change of unintended code >> execution/interpolation.
> > The difference in parsing time between single and double quotes strings is > infinitesimal
IMO there is no point in worrying about these kind of things unless the string is very long. Yves
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
To: perl5-porters [...] perl.org
Date: Tue, 18 Mar 2014 13:46:30 +0100
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Download (untitled) / with headers
text/plain 1023b
On Tue, 18 Mar 2014 13:15:11 +0100, demerphq <demerphq@gmail.com> wrote: Show quoted text
> > The difference in parsing time between single and double quotes strings is > > infinitesimal
> > IMO there is no point in worrying about these kind of things unless > the string is very long.
And not even then. I benched it years ago on several machines, and all bench results are in line-noise margins and differ per architecture. IIRC all big-endian boxes I tried on had a <1% preference for double quotes whereas little-endian boxes prefered single quotes. Let's just be consistent! More interesting would be to check if "foo: ".$foo.", bar: ".$bar is equally fast as "foo: $foo, bar: $bar". Last time I checked, the interpolated version was faster -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
On Mon Mar 17 09:40:43 2014, davem wrote: Show quoted text
> On Fri, Feb 28, 2014 at 08:59:15PM -0800, bulk88 via RT wrote:
> > I don't think I will get a response. So it stays at 5 seconds and a new > > patch mentioning 5.18.0 instead of 5.17.2 is attached.
> > Thanks, applied as af728ca1bc. I also took the libery of cleaning the code > up a bit with the following commit: > > commit 4c0e595c606032a1e1c0a922399a1e7bd2358ca9 > Author: David Mitchell <davem@iabyn.com> > AuthorDate: Mon Mar 17 16:19:10 2014 +0000 > Commit: David Mitchell <davem@iabyn.com> > CommitDate: Mon Mar 17 16:35:07 2014 +0000 > > tidy up kill0.t and kill0_child > > The previous commit added some tests to kill0.t, and added the auxiliary > file kill0_child. Tidy up the new code to better match normal standards. > In particular, improve the format, grammar and clarity of the comments, > and replace q|...| with "..." where appropriate. > Also, make the temporary filename a variable, and prefix it with "tmp-", > so that if gets left around for any reason, it's more obvious that it's > just an extraneous temporary file. > > (I haven't actually tested this commit on win32) > > >
http://www.nntp.perl.org/group/perl.daily-build.reports/2014/03/msg159824.html the test is failing on Geroge smoker. I fetched a new blead and reran it on my machine and ../t/op/kill0.t passes. The kill() calls fail. What is unusual is that there is no "not ok 9999" or out of sequence reports in the report, so I presume the child procs never got CPU time even though they got PIDs or were actually killed on an OS level and something went wrong in the win32_killpg. The only idea I have right now is to increase the sleep 5 to 15 in http://perl5.git.perl.org/perl.git/blob/HEAD:/t/op/kill0_child . -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 982b
On Fri Mar 21 13:58:09 2014, bulk88 wrote: Show quoted text
> http://www.nntp.perl.org/group/perl.daily- > build.reports/2014/03/msg159824.html the test is failing on Geroge > smoker. I fetched a new blead and reran it on my machine and > ../t/op/kill0.t passes. The kill() calls fail. What is unusual is that > there is no "not ok 9999" or out of sequence reports in the report, so > I presume the child procs never got CPU time even though they got PIDs > or were actually killed on an OS level and something went wrong in the > win32_killpg. The only idea I have right now is to increase the sleep > 5 to 15 in > http://perl5.git.perl.org/perl.git/blob/HEAD:/t/op/kill0_child .
I sort of (no CC no make, tests that use either fail catasfrical (not a word, i kno), M::B hangs forever on nearly every test) reran blead on a win2k machine. nothing kill related failed and the kill .ts didnt fail. So its something specific to a VM or the george software stack. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 222b
bulk88 requested that this be marked a 5.20.0 blocker, but I'm not entirely sure what issue remains on the ticket, on which discussion has sort of meandered. (a) is this still a blocker candidate? (b) what's up? -- rjbs
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Date: Mon, 24 Mar 2014 16:07:18 +0000
To: Ricardo SIGNES via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 683b
On Mon, Mar 24, 2014 at 07:19:37AM -0700, Ricardo SIGNES via RT wrote: Show quoted text
> bulk88 requested that this be marked a 5.20.0 blocker, but I'm not entirely sure what issue remains on the ticket, on which discussion has sort of meandered. > > (a) is this still a blocker candidate? > (b) what's up?
The bug has been fixed, but the just-added test is failing on George's smoker, but not for bulk88. So we either need to get the test fixed, or backout the new test. -- More than any other time in history, mankind faces a crossroads. One path leads to despair and utter hopelessness. The other, to total extinction. Let us pray we have the wisdom to choose correctly. -- Woody Allen
From: bulk88 <bulk88 [...] hotmail.com>
CC: Ricardo SIGNES via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2,regression 5.18
Date: Mon, 24 Mar 2014 17:58:08 -0400
Download (untitled) / with headers
text/plain 903b
Dave Mitchell wrote: Show quoted text
> On Mon, Mar 24, 2014 at 07:19:37AM -0700, Ricardo SIGNES via RT wrote:
>> bulk88 requested that this be marked a 5.20.0 blocker, but I'm not entirely sure what issue remains on the ticket, on which discussion has sort of meandered. >> >> (a) is this still a blocker candidate? >> (b) what's up?
> > The bug has been fixed, but the just-added test is failing on George's > smoker, but not for bulk88. > > So we either need to get the test fixed, or backout the new test. >
5.18 maint backport is also a possibility since this is a regression for a feature nobody knew existed, except for probably demerphq (who wrote IIRC) and he doesnt use Windows anymore. But first figuring out why the George smoker failed is required. Steve Hay, Tony, can you 2 trying building Win32 Perl and see what happens? There arent any active Win32 porters other than me, Hay and Cook AFAIK.
From: Steve Hay <steve.m.hay [...] googlemail.com>
CC: Ricardo SIGNES via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2,regression 5.18
To: bulk88 <bulk88 [...] hotmail.com>
Date: Mon, 24 Mar 2014 23:09:13 +0000
Download (untitled) / with headers
text/plain 2.1k
On 24 March 2014 21:58, bulk88 <bulk88@hotmail.com> wrote: Show quoted text
> Dave Mitchell wrote:
>> >> On Mon, Mar 24, 2014 at 07:19:37AM -0700, Ricardo SIGNES via RT wrote:
>>> >>> bulk88 requested that this be marked a 5.20.0 blocker, but I'm not >>> entirely sure what issue remains on the ticket, on which discussion has sort >>> of meandered. >>> >>> (a) is this still a blocker candidate? >>> (b) what's up?
>> >> >> The bug has been fixed, but the just-added test is failing on George's >> smoker, but not for bulk88. >> >> So we either need to get the test fixed, or backout the new test. >>
> > 5.18 maint backport is also a possibility since this is a regression for a > feature nobody knew existed, except for probably demerphq (who wrote IIRC) > and he doesnt use Windows anymore. But first figuring out why the George > smoker failed is required. Steve Hay, Tony, can you 2 trying building Win32 > Perl and see what happens? There arent any active Win32 porters other than > me, Hay and Cook AFAIK.
Sorry, I had assumed that I couldn't reproduce either this since my recent perl build/tests have all been happily passing, but on closer inspection I see that George's smoker only fails without useithreads. So I built with USE_MULTI=undef USE_ITHREADS=undef USE_IMP_SYS=undef and sure enough it fails: C:\Dev\Git\perl\t>..\perl harness -v op\kill0.t op/kill0.t .. 1..9 ok 1 - kill(0, $pid) returns true if $pid exists ok 2 - kill(0, $pid) returns false if $pid does not exist ok 3 - dies killing undef pid ok 4 - dies killing empty string pid ok 5 - dies killing alphabetic pid ok 6 - can kill a number string in a magic variable # Failed test 7 - process group kill, named signal at op/kill0.t line 82 # got "0" # expected "1" not ok 7 - process group kill, named signal ok 8 - $Config{sig_name} parsed correctly # Failed test 9 - process group kill, numeric signal at op/kill0.t line 107 # got "0" # expected "1" not ok 9 - process group kill, numeric signal Failed 2/9 subtests Test Summary Report ------------------- op/kill0.t (Wstat: 0 Tests: 9 Failed: 2) Failed tests: 7, 9 Files=1, Tests=9, 2 wallclock secs ( 0.05 usr + 0.00 sys = 0.05 CPU) Result: FAIL I don't have time to look right now, but will dig deeper tomorrow...
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Mon Mar 24 16:09:44 2014, shay wrote: Show quoted text
> Sorry, I had assumed that I couldn't reproduce either this since my > recent perl build/tests have all been happily passing, but on closer > inspection I see that George's smoker only fails without useithreads. > > So I built with USE_MULTI=undef USE_ITHREADS=undef USE_IMP_SYS=undef > and sure enough it fails:
Reproduced. ----------------------------------------- C:\perl519\src\t>..\perl -I..\lib op/kill0.t 1..9 ok 1 - kill(0, $pid) returns true if $pid exists ok 2 - kill(0, $pid) returns false if $pid does not exist ok 3 - dies killing undef pid ok 4 - dies killing empty string pid ok 5 - dies killing alphabetic pid ok 6 - can kill a number string in a magic variable not ok 7 - process group kill, named signal # Failed test 7 - process group kill, named signal at op/kill0.t line 82 # got "0" # expected "1" ok 8 - $Config{sig_name} parsed correctly not ok 9 - process group kill, numeric signal # Failed test 9 - process group kill, numeric signal at op/kill0.t line 107 # got "0" # expected "1" C:\perl519\src\t> ----------------------------------------- I will investigate. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 576b
On Mon Mar 24 17:11:39 2014, bulk88 wrote: Show quoted text
> > I will investigate.
Perl's win32 killpg in win32.c doesn't match the POSIX expectations that Perl_apply has for killpg. When PERL_IMPLICIT_SYS/perlhost.h/PerlProcKillpg is removed, then Perl_apply directly calls killpg which follows Win32 coding style, not POSIX coding style. With PERL_IMPLICIT_SYS, PerlProcKillpg calls win32_kill which calls my_kill which calls killpg. I plan on renaming killpg to my_killpg, then create a win32_killpg that follows POSIXese, and untangle the spaghetti. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 412b
On Mon Mar 24 18:00:26 2014, bulk88 wrote: Show quoted text
> I plan on renaming > killpg to my_killpg, then create a win32_killpg that follows POSIXese, > and untangle the spaghetti.
Code already written but I can't make test it ATM. Will take a day or 2 to get to posting the patch, need to file another Perl RT or CPAN RT bug first since make test fails badly for me due to a /cpan module. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 590b
On Tue Mar 25 22:52:13 2014, bulk88 wrote: Show quoted text
> On Mon Mar 24 18:00:26 2014, bulk88 wrote:
> > I plan on renaming > > killpg to my_killpg, then create a win32_killpg that follows > > POSIXese, > > and untangle the spaghetti.
> > Code already written but I can't make test it ATM. Will take a day or > 2 to get to posting the patch, need to file another Perl RT or CPAN RT > bug first since make test fails badly for me due to a /cpan module.
kill0.t passed for me on no-threads perl with this patch. I couldn't make harness finish due to a different bug. -- bulk88 ~ bulk88 at hotmail.com
Subject: 0001-fix-killpg-on-Win32-to-meet-posix-expectations-for-k.patch
From 5916dd7a8033a4d89aa7f6d7e52a718e96a7fa4d Mon Sep 17 00:00:00 2001 From: Daniel Dragan <bulk88@hotmail.com> Date: Thu, 27 Mar 2014 03:37:03 -0400 Subject: [PATCH] fix killpg on Win32, to meet posix expectations for killpg On Win32 Perls built without PERL_IMPLICIT_SYS, killpg from win32.c was directly called by Perl_apply, yet killpg's return value had Win32 behavior, not POSIX behavior. Modify killpg token to have same meaning as PerlProcKillpg/PerlProc_killpg has on PERL_IMPLICIT_SYS builds. Use a macro rather than create a win32_killpg C function since win32_killpg would be nothing but a call to win32_kill anyways. win32_kill contains the Win32 to POSIX semantics conversion code. Rename old killpg to my_killpg since it has no use outside of win32.c. The psuedo-PID code in win32_kill also played a factor in not writing a separate win32_killpg that calls my_killpg. This fix is tested by kill0.t passing on no-PERL_IMPLICIT_SYS builds. [perl #121230] --- pod/perldelta.pod | 8 ++++++++ win32/win32.c | 11 +++++++---- win32/win32.h | 1 - win32/win32iop.h | 5 +++++ 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index 358aea3..aa7e1ce 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -316,6 +316,14 @@ a bug in which the timeout monitor used for tests could not be cancelled once the test completes, and the full timeout period elapsed before running the next test file. +On a Perl built without psuedo-fork (psuedo-fork builds were not affected by +this bug), probably since prcess tree kill feature was implemented on Win32, +killing a process tree with L<perlfunc/kill> and a negative signal, resulted +in kill inverting the returned value. This ment successfully killing +1 process tree PID returned 0, and also passing 2 invalid PID, returned 2. +This has been corrected so the documented behavior for return values for kill +executes. [perl #121230] + =back =head1 Internal Changes diff --git a/win32/win32.c b/win32/win32.c index dda951b..6dc6ec1 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -139,6 +139,7 @@ static int do_spawnvp_handles(int mode, const char *cmdname, static long find_pid(pTHX_ int pid); static void remove_dead_process(long child); static int terminate_process(DWORD pid, HANDLE process_handle, int sig); +static int my_killpg(int pid, int sig); static int my_kill(int pid, int sig); static void out_of_memory(void); static char* wstr_to_str(const wchar_t* wstr); @@ -1250,8 +1251,9 @@ terminate_process(DWORD pid, HANDLE process_handle, int sig) return 0; } -int -killpg(int pid, int sig) +/* returns number of processes killed */ +static int +my_killpg(int pid, int sig) { HANDLE process_handle; HANDLE snapshot_handle; @@ -1271,7 +1273,7 @@ killpg(int pid, int sig) if (Process32First(snapshot_handle, &entry)) { do { if (entry.th32ParentProcessID == (DWORD)pid) - killed += killpg(entry.th32ProcessID, sig); + killed += my_killpg(entry.th32ProcessID, sig); entry.dwSize = sizeof(entry); } while (Process32Next(snapshot_handle, &entry)); @@ -1282,6 +1284,7 @@ killpg(int pid, int sig) return killed; } +/* returns number of processes killed */ static int my_kill(int pid, int sig) { @@ -1289,7 +1292,7 @@ my_kill(int pid, int sig) HANDLE process_handle; if (sig < 0) - return killpg(pid, -sig); + return my_killpg(pid, -sig); process_handle = OpenProcess(PROCESS_TERMINATE, FALSE, pid); /* OpenProcess() returns NULL on error, *not* INVALID_HANDLE_VALUE */ diff --git a/win32/win32.h b/win32/win32.h index 3d1655a..e109939 100644 --- a/win32/win32.h +++ b/win32/win32.h @@ -322,7 +322,6 @@ extern gid_t getegid(void); extern int setuid(uid_t uid); extern int setgid(gid_t gid); extern int kill(int pid, int sig); -extern int killpg(int pid, int sig); #ifndef USE_PERL_SBRK extern void *sbrk(ptrdiff_t need); # define HAS_SBRK_PROTO diff --git a/win32/win32iop.h b/win32/win32iop.h index 11d4219..246375f 100644 --- a/win32/win32iop.h +++ b/win32/win32iop.h @@ -448,6 +448,11 @@ END_EXTERN_C # undef kill #endif #define kill win32_kill +#ifdef UNDER_CE +# undef killpg +#endif +#define killpg(pid, sig) win32_kill(pid, -(sig)) + #ifdef UNDER_CE # undef opendir -- 1.7.9.msysgit.0
CC: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
To: Tony Cook via RT <perlbug-followup [...] perl.org>
Date: Thu, 27 Mar 2014 13:00:49 +0000
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 825b
On 27 March 2014 07:41, bulk88 via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue Mar 25 22:52:13 2014, bulk88 wrote:
>> On Mon Mar 24 18:00:26 2014, bulk88 wrote:
>> > I plan on renaming >> > killpg to my_killpg, then create a win32_killpg that follows >> > POSIXese, >> > and untangle the spaghetti.
>> >> Code already written but I can't make test it ATM. Will take a day or >> 2 to get to posting the patch, need to file another Perl RT or CPAN RT >> bug first since make test fails badly for me due to a /cpan module.
> > > kill0.t passed for me on no-threads perl with this patch. I couldn't make harness finish due to a different bug.
Thanks, applied as 721b26746b. (A full "nmake test 2>nul" with no PERL_IMPLICIT_SYS passed all tests for me.) Does this complete #121230 now? I.e. should the ticket be closed now?
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 627b
On Thu Mar 27 06:01:15 2014, shay wrote: Show quoted text
> On 27 March 2014 07:41, bulk88 via RT <perlbug-followup@perl.org>
> > > > kill0.t passed for me on no-threads perl with this patch. I couldn't > > make harness finish due to a different bug.
> > Thanks, applied as 721b26746b. (A full "nmake test 2>nul" with no > PERL_IMPLICIT_SYS passed all tests for me.) > > Does this complete #121230 now? I.e. should the ticket be closed now?
Lets wait until the George smoker delivers a new report. I see nothing since Mar 23 on the Win2k smoker http://www.nntp.perl.org/group/perl.daily-build.reports/ . -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 785b
On Thu Mar 27 11:02:51 2014, bulk88 wrote: Show quoted text
> On Thu Mar 27 06:01:15 2014, shay wrote:
> > On 27 March 2014 07:41, bulk88 via RT <perlbug-followup@perl.org>
> > > > > > kill0.t passed for me on no-threads perl with this patch. I > > > couldn't > > > make harness finish due to a different bug.
> > > > Thanks, applied as 721b26746b. (A full "nmake test 2>nul" with no > > PERL_IMPLICIT_SYS passed all tests for me.) > > > > Does this complete #121230 now? I.e. should the ticket be closed now?
> > Lets wait until the George smoker delivers a new report. I see nothing > since Mar 23 on the Win2k smoker > http://www.nntp.perl.org/group/perl.daily-build.reports/ .
Mar 23 is still the last smoke. I think it hung. Its been 12 days with no response. -- bulk88 ~ bulk88 at hotmail.com
RT-Send-CC: perl5-porters [...] perl.org
On Thu Apr 03 23:27:06 2014, bulk88 wrote: Show quoted text
> On Thu Mar 27 11:02:51 2014, bulk88 wrote:
> > On Thu Mar 27 06:01:15 2014, shay wrote:
> > > On 27 March 2014 07:41, bulk88 via RT <perlbug-followup@perl.org>
> > > > > > > > kill0.t passed for me on no-threads perl with this patch. I > > > > couldn't > > > > make harness finish due to a different bug.
> > > > > > Thanks, applied as 721b26746b. (A full "nmake test 2>nul" with no > > > PERL_IMPLICIT_SYS passed all tests for me.) > > > > > > Does this complete #121230 now? I.e. should the ticket be closed > > > now?
> > > > Lets wait until the George smoker delivers a new report. I see > > nothing > > since Mar 23 on the Win2k smoker > > http://www.nntp.perl.org/group/perl.daily-build.reports/ .
> > Mar 23 is still the last smoke. I think it hung. Its been 12 days > with no response.
George the human responded about problems with the process group kill code in Message-ID: <alpine.LFD.2.11.1404222133090.31829@ein.m-l.org> . Its not showing up on nttp.perl.org yet so I can't offer a link. -- bulk88 ~ bulk88 at hotmail.com
Date: Wed, 23 Apr 2014 10:47:50 +0100
To: perl5-porters [...] perl.org
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
From: Smylers <Smylers [...] stripey.com>
Download (untitled) / with headers
text/plain 724b
bulk88 via RT writes: Show quoted text
> George the human responded about problems with the process group kill > code in Message-ID: <alpine.LFD.2.11.1404222133090.31829@ein.m-l.org> > . Its not showing up on nttp.perl.org yet so I can't offer a link.
It's http://nntp.perl.org/group/perl.perl5.porters/214650 For future reference, all P5P emails contain a link to their own web archive page. View full headers on the above message and you'll see: X-List-Archive: <http://nntp.perl.org/group/perl.perl5.porters/214650> Smylers -- Why drug companies shouldn't be allowed to hide research results that they don't like: http://gu.com/p/3zat8 — UK gov wasted millions on Tamiflu Sign the AllTrials petition: http://www.alltrials.net/
CC: Perl5 Porteros <perl5-porters [...] perl.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Date: Wed, 23 Apr 2014 13:34:21 +0200
To: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1.2k
On 18 March 2014 13:46, H.Merijn Brand <h.m.brand@xs4all.nl> wrote: Show quoted text
> On Tue, 18 Mar 2014 13:15:11 +0100, demerphq <demerphq@gmail.com> wrote: >
>> > The difference in parsing time between single and double quotes strings is >> > infinitesimal
>> >> IMO there is no point in worrying about these kind of things unless >> the string is very long.
> > And not even then. I benched it years ago on several machines, and all > bench results are in line-noise margins and differ per architecture. > IIRC all big-endian boxes I tried on had a <1% preference for double > quotes whereas little-endian boxes prefered single quotes.
Well no, when the string becomes long it starts being interesting to not use a quoted string, or to use single quoted here docs. When I checked last Perl has non-linear and degrading performance parsing strings. The longer the string the slower it gets. In practice unless the string is quite long (hundreds to thousands of characters) it is not something you need to worry about at all. On the other hand if you happen to be dealing with a case where you are generating code with lots of long quoted strings you can see real parsing performance gains by removing the strings. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Date: Wed, 23 Apr 2014 09:17:07 -0400
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 323b
* Smylers <Smylers@stripey.com> [2014-04-23T05:47:50] Show quoted text
> For future reference, all P5P emails contain a link to their own web > archive page. View full headers on the above message and you'll see: > > X-List-Archive: <http://nntp.perl.org/group/perl.perl5.porters/214650>
<jawdrop> You're my hero of the day! -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

From: George Greer <perl [...] greerga.m-l.org>
To: bulk88 via RT <perlbug-followup [...] perl.org>
Date: Wed, 23 Apr 2014 09:56:45 -0400 (EDT)
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
On Wed, 23 Apr 2014, bulk88 via RT wrote: Show quoted text
> On Thu Apr 03 23:27:06 2014, bulk88 wrote:
>> On Thu Mar 27 11:02:51 2014, bulk88 wrote:
>>> On Thu Mar 27 06:01:15 2014, shay wrote:
>>>> On 27 March 2014 07:41, bulk88 via RT <perlbug-followup@perl.org>
>>>>> >>>>> kill0.t passed for me on no-threads perl with this patch. I >>>>> couldn't >>>>> make harness finish due to a different bug.
>>>> >>>> Thanks, applied as 721b26746b. (A full "nmake test 2>nul" with no >>>> PERL_IMPLICIT_SYS passed all tests for me.) >>>> >>>> Does this complete #121230 now? I.e. should the ticket be closed >>>> now?
>>> >>> Lets wait until the George smoker delivers a new report. I see >>> nothing >>> since Mar 23 on the Win2k smoker >>> http://www.nntp.perl.org/group/perl.daily-build.reports/ .
>> >> Mar 23 is still the last smoke. I think it hung. Its been 12 days >> with no response.
> > George the human responded about problems with the process group kill code in Message-ID: <alpine.LFD.2.11.1404222133090.31829@ein.m-l.org> . Its not showing up on nttp.perl.org yet so I can't offer a link.
Archives of the logs and reports are always on my web site: http://m-l.org/~perl/smoke/perl/win32/blead/ Yes, the process group kill code is probably nuking the Win32 blead smoker. The maint and smoke-me smokers haven't been killed, so the impact is limited but still annoying. (Kind of like the test in blead that is hanging and I manually kill every so often.) As for the reports, I had a hard drive go bad in the RAID5 so I shut the machine off until the warranty replacement arrived. I updated the status pages[1] but didn't mail the list as it wasn't "interesting enough". 1: http://m-l.org/~perl/smoke/perl/win32/blead/status http://m-l.org/~perl/smoke/perl/linux/blead_g++_quick/status ...etc... -- George Greer
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Date: Wed, 23 Apr 2014 20:44:42 +0100
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
CC: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 688b
On 23 April 2014 14:17, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote: Show quoted text
> * Smylers <Smylers@stripey.com> [2014-04-23T05:47:50]
>> For future reference, all P5P emails contain a link to their own web >> archive page. View full headers on the above message and you'll see: >> >> X-List-Archive: <http://nntp.perl.org/group/perl.perl5.porters/214650>
> > <jawdrop> > > You're my hero of the day! >
This was news to me too, and would have saved a delay in updating the epigraphs.pod after the release of 5.19.11 had I known before. I've therefore updated the RMG with this useful information: http://perl5.git.perl.org/perl.git/commit/70d95cc994e515d77711476f6c853c3f1f1f1458 Thanks!
From: Dave Mitchell <davem [...] iabyn.com>
CC: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>, Perl5 Porteros <perl5-porters [...] perl.org>
Date: Thu, 24 Apr 2014 15:54:11 +0100
To: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #121230] process group kill on Win32 broken in 5.17.2, regression 5.18
Download (untitled) / with headers
text/plain 1.6k
On Wed, Apr 23, 2014 at 01:34:21PM +0200, demerphq wrote: Show quoted text
> On 18 March 2014 13:46, H.Merijn Brand <h.m.brand@xs4all.nl> wrote:
> > On Tue, 18 Mar 2014 13:15:11 +0100, demerphq <demerphq@gmail.com> wrote: > >
> >> > The difference in parsing time between single and double quotes strings is > >> > infinitesimal
> >> > >> IMO there is no point in worrying about these kind of things unless > >> the string is very long.
> > > > And not even then. I benched it years ago on several machines, and all > > bench results are in line-noise margins and differ per architecture. > > IIRC all big-endian boxes I tried on had a <1% preference for double > > quotes whereas little-endian boxes prefered single quotes.
> > Well no, when the string becomes long it starts being interesting to > not use a quoted string, or to use single quoted here docs. > > When I checked last Perl has non-linear and degrading performance > parsing strings. The longer the string the slower it gets. > > In practice unless the string is quite long (hundreds to thousands of > characters) it is not something you need to worry about at all. On the > other hand if you happen to be dealing with a case where you are > generating code with lots of long quoted strings you can see real > parsing performance gains by removing the strings.
The double-quoted parsing code is very inefficient. It does a lot of checks for each char position in the string; I suspect we could speed it up tremendously by doing a strchr() or similar to skip to the next "interesting" char, and only then doing all the checks. -- It's not that I'm afraid to die, I just don't want to be there when it happens. -- Woody Allen


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org