Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win32 Perl's signal emulation permanently disables clib signals, then sets them, and excessive win32_signal_context() calls #12679

Closed
p5pRT opened this issue Dec 31, 2012 · 5 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 31, 2012

Migrated from rt.perl.org#116254 (status was 'open')

Searchable as RT116254$

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2012

From @bulk88

Created by @bulk88

I noticed a redundant call to win32_signal_context from Perl_csighandler
when doing a ctrl-c

________________________________________________________________________________

perl517.dll!win32_signal_context() Line 4168 C
  perl517.dll!Perl_csighandler(int sig=2) Line 1310 + 0x5 C
  perl517.dll!do_raise(interpreter * my_perl=0x00346014, int sig=2)
Line 2125 + 0x7 C
  perl517.dll!win32_ctrlhandler(unsigned long dwCtrlType=0) Line
4205 + 0xb C
  kernel32.dll!_CtrlRoutine@​4() + 0x118
  kernel32.dll!_BaseThreadStart@​8() + 0x37
________________________________________________________________________________

Yet TLS is guarenteed to have an interp by now because of
win32_ctrlhandler also calling win32_signal_context(). Now, perl does
register a sig handler with the CRT
________________________________________________________________________________

msvcr71.dll!signal(int signum=2, void (int)* sigact=0x28144c40)
Line 227 C
  perl517.dll!win32_signal(int sig=2, void (int)* subcode=0x28144c40)
Line 4402 + 0xe C
  perl517.dll!PerlProcSignal(IPerlProc * piPerl=0x003445c0, int sig=2,
void (int)* subcode=0x28144c40) Line 1681 + 0xd C
  perl517.dll!Perl_rsignal(interpreter * my_perl=0x00346014, int
signo=2, void (int)* handler=0x28144c40) Line 3013 + 0x1e C
  perl517.dll!Perl_magic_setsig(interpreter * my_perl=0x00346014, sv *
sv=0x00944884, magic * mg=0x00951184) Line 1536 + 0x14 C
  perl517.dll!Perl_mg_set(interpreter * my_perl=0x00346014, sv *
sv=0x00944884) Line 279 + 0x12 C
  perl517.dll!Perl_pp_sassign(interpreter * my_perl=0x00346014) Line
223 + 0x3c C
  perl517.dll!Perl_runops_standard(interpreter * my_perl=0x00346014)
Line 42 + 0xa C
  perl517.dll!S_run_body(interpreter * my_perl=0x00346014, long
oldscope=1) Line 2430 + 0xd C
  perl517.dll!perl_run(interpreter * my_perl=0x00346014) Line 2349 C
  perl517.dll!RunPerl(int argc=2, char * * argv=0x00342478, char * *
env=0x00345308) Line 270 + 0x9 C
  perl.exe!mainCRTStartup() Line 398 + 0xe C
  kernel32.dll!_BaseProcessStart@​4() + 0x23
____________________________________________________________________________________

but, from my research, a CRT sig handler will never be called, except
through maybe raise(), which win32 perl does not link with (I checked
import table of perl517.dll), the first time signal() is called in the
process, the CRT sets a flag inside itself that it itself called
SetConsoleCtrlHandler, it will never call it again. When Perl removes
the CRT handler, and installs its own, it has disabled CRT signals for
the rest of the process's run. Yet perl continues to register sig
handlers with the CRT even though they never will be called by the OS.

relevant code is

signal(SIGINT,win32_csighandler);

I feel something is redundant here or should be removed. I am not sure
what. If Perl_csighandler can only be called with raise (same thread),
or from win32_ctrlhandler (random OS thread), it needs a normal dTHX,
not a dTHXa(PERL_GET_SIG_CONTEXT);. Maybe Perl should stop registering
Perl_csighandler with the CRT with signal() since that handler will
never be called from the OS, or Perl.

Trying to synthesis a ctrl-c with kill didn't work (
http​://perlmonks.org/?node_id=1010891 not sure if this will work on Unix
or not). On some signums, it killed the perl process (NUM05) in

terminate_process(DWORD pid, HANDLE process_handle, int sig)
, on others the %SIG was never called (like BREAK) (IDK why yet on BREAK).

Also Perl_sighandler calls win32_signal_context, yet can only be called
from Perl_csighandler or Perl_despatch_signals, both of which already
called win32_signal_context (Perl_csighandler) or always have TLS set up
correctly (Perl_despatch_signals).

Perl's Win32 emulated signals will never be posix compatible, but they
shouldn't do a bad job at what limited capability they have. I'd like
some comments before I try changing any of this code and writing a patch.

Perl Info

Flags:
     category=core
     severity=low

Site configuration information for perl 5.17.7:

Configured by Owner at Sun Dec 16 13:25:34 2012.

Summary of my perl5 (revision 5 version 17 subversion 7 patch blead 
2012-12-06.16:42:20 93a641ae382638ffd1980378be4810244d04f4b0 
v5.17.6-186-g93a641a) configuration:
   Snapshot of: 93a641ae382638ffd1980378be4810244d04f4b0
   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 -MD -Zi -DNDEBUG -O1 -GL -G7 
-DWIN32 -D_CONSOLE -DNO_STRICT  -DPERL_TEXTMODE_SCRIPTS 
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO 
-D_USE_32BIT_TIME_T',
     optimize='-MD -Zi -DNDEBUG -O1 -GL -G7',
     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:\perl517\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=perl517.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:\perl517\lib\CORE"  -machine:x86'

Locally applied patches:



@INC for perl 5.17.7:
     C:/perl517/site/lib
     C:/perl517/lib
     .


Environment for perl 5.17.7:
     HOME (unset)
     LANG (unset)
     LANGUAGE (unset)
     LD_LIBRARY_PATH (unset)
     LOGDIR (unset)
     PATH=C:\perl517\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)

@p5pRT
Copy link
Author

p5pRT commented Aug 20, 2013

From @tonycoz

On Mon Dec 31 00​:53​:30 2012, bulk88 wrote​:

This is a bug report for perl from bulk88@​hotmail.com,
generated with the help of perlbug 1.39 running under perl 5.17.7.

-----------------------------------------------------------------
[Please describe your issue here]

I noticed a redundant call to win32_signal_context from
Perl_csighandler
when doing a ctrl-c

________________________________________________________________________________

perl517.dll!win32_signal_context() Line 4168 C
perl517.dll!Perl_csighandler(int sig=2) Line 1310 + 0x5 C
perl517.dll!do_raise(interpreter * my_perl=0x00346014, int sig=2)
Line 2125 + 0x7 C
perl517.dll!win32_ctrlhandler(unsigned long dwCtrlType=0) Line
4205 + 0xb C
kernel32.dll!_CtrlRoutine@​4() + 0x118
kernel32.dll!_BaseThreadStart@​8() + 0x37

________________________________________________________________________________

Yet TLS is guarenteed to have an interp by now because of
win32_ctrlhandler also calling win32_signal_context().

I'm not sure that's true, do_raise() is also called from
win32_process_message()
which (if I understand the code) may be called in the context of a different
thread.

Now, perl does

register a sig handler with the CRT
...

but, from my research, a CRT sig handler will never be called, except
through maybe raise(), which win32 perl does not link with (I checked
import table of perl517.dll), the first time signal() is called in the
process, the CRT sets a flag inside itself that it itself called
SetConsoleCtrlHandler, it will never call it again. When Perl removes
the CRT handler, and installs its own, it has disabled CRT signals for
the rest of the process's run. Yet perl continues to register sig
handlers with the CRT even though they never will be called by the OS.

relevant code is

http​://perl5.git.perl.org/perl.git/blob/1feab43956f936b2526693e4394a9653fcea6079​:/win32/win32.c#l4570

I feel something is redundant here or should be removed. I am not sure
what. If Perl_csighandler can only be called with raise (same thread),
or from win32_ctrlhandler (random OS thread), it needs a normal dTHX,
not a dTHXa(PERL_GET_SIG_CONTEXT);. Maybe Perl should stop registering
Perl_csighandler with the CRT with signal() since that handler will
never be called from the OS, or Perl.

Perhaps it's to allow interoperability with XS modules that call raise().

I can't think of any other reason.

Perhaps that's why Perl_sys_inter_init() calls signal() - so XS calling
signal() won't interfere with perl's initialization of the CTRL-C
processing flag.

Trying to synthesis a ctrl-c with kill didn't work (
http​://perlmonks.org/?node_id=1010891 not sure if this will work on
Unix
or not). On some signums, it killed the perl process (NUM05) in

http​://perl5.git.perl.org/perl.git/blob/1feab43956f936b2526693e4394a9653fcea6079​:/win32/win32.c#l1231

, on others the %SIG was never called (like BREAK) (IDK why yet on
BREAK).

What brought me to this ticket was this issue.

I was attemptting to write a signal handler test for #85104, but kill
"INT", $pid wasn't working from a separate process.

I did end up managing to generate a signal caught by
$SIG{INT}/$SIG{BREAK}, but using a C program that calls
GenerateConsoleCtrlEvent() with a dwProcessGroupId of 0, which I can't
see a way of doing from perl code (kill 'INT', 0 fails because it can't
open pid 0.)

Supplying the pid of the process to GenerateConsoleCtrlEvent() never
worked (and failed when called from a different console.)

This appears to be a Win32 issue (ie. Windows is strange...)

Testing done on Windows 7.

Also Perl_sighandler calls win32_signal_context, yet can only be
called
from Perl_csighandler or Perl_despatch_signals, both of which already
called win32_signal_context (Perl_csighandler) or always have TLS set
up
correctly (Perl_despatch_signals).

See my note about win32_process_message() above.

Perl's Win32 emulated signals will never be posix compatible, but they
shouldn't do a bad job at what limited capability they have. I'd like
some comments before I try changing any of this code and writing a
patch.

Tests are probably the hard part.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 20, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Oct 6, 2015

From @jkeenan

bulk88​: If I read the comments in this ticket correctly -- and I concede I might not, because the way it's displaying in RT is somewhat confusing -- TonyC responded in Aug 2013 to your initial post and there has been no correspondence since.

Is this still an ongoing concern? If not, then I think we should close the ticket.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@toddr
Copy link
Member

toddr commented Feb 13, 2020

@bulk88 given no response in 4 years, I'm closing this.

@toddr toddr closed this as completed Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants