Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.93k stars 551 forks source link

Overriding built-ins for thread safety is done in an incredibly confusing way #11886

Open p5pRT opened 12 years ago

p5pRT commented 12 years ago

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

Searchable as RT108762$

p5pRT commented 12 years ago

From @Leont

This is a bug report for perl from fawaka@​gmail.com\, generated with the help of perlbug 1.39 running under perl 5.14.2.


A lot of perl's built-ins and some core modules are based on POSIX APIs that are listed as not guaranteeing to be thread-safe. This means that implementations are not required to make these functions thread-safe (though most could be made thread-safe using thread local storage). I doubt there's any non-trivial program using threads that's not using any of these. In particular\, this includes​:

* drand48() [ rand ] * dlerror() [ DynaLoader ] * endgrent() * endpwent() * getenv() [ %ENV ] * getgrent() * getgrgid() * getgrnam() * gethostbyaddr() * gethostbyname() * gethostent() * getlogin() * getnetbyaddr() * getnetbyname() * getnetent() * getprotobyname() * getprotobynumber() * getprotoent() * getpwent() * getpwnam() * getpwuid() * getservbyname() * getservbyport() * getservent() * localeconv() [ POSIX ] * nl_langinfo() [ I18N​::LangInfo ] * readdir() * setgrent() * setpwent() * strerror() [ $!\, $^E ]

Most of these functions have thread-safe/reentrant equivalents (e.g. gethostbyname_r).

Another function uses an internal reimplementation that suffers from the same inherent issues with regard to signal handling

* system() (its signal handling is a bit problematic in presense of threads).

Some of the unsafe functions in POSIX are provided as built-ins\, but are already implemented some other thread-safe/reentrant implementations​:

* localtime() * gmtime() * inet_ntoa() [Socket.pm] * Two more is simply ignored outside the main thread​:

* setenv [ %ENV ] * unsetenv [ %ENV ]



Flags​:   category=core   severity=low


Site configuration information for perl 5.14.2​:

Configured by leon at Tue Jan 17 21​:29​:06 CET 2012.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration​:

  Platform​:   osname=linux\, osvers=3.0.0-14-generic\, archname=x86_64-linux-thread-multi   uname='linux leon-laptop 3.0.0-14-generic #23-ubuntu smp mon nov 21 20​:28​:43 utc 2011 x86_64 x86_64 x86_64 gnulinux '   config_args='-de -Dprefix=/home/leon/perl5/perlbrew/perls/perl-5.14.2 -Dusethreads'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=define\, usemultiplicity=define   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=define\, use64bitall=define\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\,   optimize='-O2'\,   cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.6.1'\, gccosandvers=''   intsize=4\, longsize=8\, ptrsize=8\, doublesize=8\, byteorder=12345678   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags =' -fstack-protector -L/usr/local/lib'   libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib   libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lpthread -lc   perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc   libc=\, so=so\, useshrplib=false\, libperl=libperl.a   gnulibc_version='2.13'   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E'   cccdlflags='-fPIC'\, lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches​:


@​INC for perl 5.14.2​:   /home/leon/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/x86_64-linux-thread-multi   /home/leon/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2   /home/leon/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2/x86_64-linux-thread-multi   /home/leon/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2   .


Environment for perl 5.14.2​:   HOME=/home/leon   LANG=en_US.utf8   LANGUAGE=en_US​:en_GB​:en   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/home/leon/perl5/perlbrew/bin​:/home/leon/perl5/perlbrew/perls/perl-5.14.2/bin​:/home/leon/bin​:/usr/lib/lightdm/lightdm​:/usr/local/sbin​:/usr/local/bin​:/usr/sbin​:/usr/bin​:/sbin​:/bin​:/usr/games   PERLBREW_HOME=/home/leon/.perlbrew   PERLBREW_PATH=/home/leon/perl5/perlbrew/bin​:/home/leon/perl5/perlbrew/perls/perl-5.14.2/bin   PERLBREW_PERL=perl-5.14.2   PERLBREW_ROOT=/home/leon/perl5/perlbrew   PERLBREW_VERSION=0.25   PERL_BADLANG (unset)   SHELL=/bin/bash

p5pRT commented 12 years ago

From @craigberry

On Sun\, Jan 22\, 2012 at 9​:30 AM\, Leon Timmermans \perlbug\-followup@​perl\.org wrote​:

# New Ticket Created by  Leon Timmermans # Please include the string​:  [perl #108762] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=108762 >

This is a bug report for perl from fawaka@​gmail.com\, generated with the help of perlbug 1.39 running under perl 5.14.2.

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

A lot of perl's built-ins and some core modules are based on POSIX APIs that are listed as not guaranteeing to be thread-safe.

\

Most of these functions have thread-safe/reentrant equivalents (e.g. gethostbyname_r).

But doesn't Perl use those if available (or advisable\, since IIRC some of the platforms that introduced such functions later deprecated them and made the non-_r versions thread safe)?

Not that everything needing attention has gotten as much as it needs. Please do have a whack at regen/reentrl.pl and post patches about what's out-of-date or broken. The most salient comment from that file seems to be​:

/* If compiling for a threaded perl\, we will macro-wrap the system/library * interfaces (e.g. getpwent()) which have threaded versions * (e.g. getpwent_r())\, which will handle things correctly for * the Perl interpreter\, but otherwise (for XS) the wrapping does * not take place. See L\<perlxs/Thread-aware system interfaces>. */

My impression (based on kinda sorta paying attention to people who actually know things) is that thread safety in the core is most likely to run aground on the combination of threads\, signals\, and the stack manipulations via which the VM does its thing. Which isn't to say things *can't* go wrong when calling POSIX libraries\, but just that they don't present the biggest window for mayhem\, and when they do\, it's more likely to be a platform/porting problem than a core architecture problem.

But you probably know at least some of this and I've probably missed at least some of the point of your bug report\, so please elaborate.

p5pRT commented 12 years ago

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

p5pRT commented 12 years ago

From @Leont

On Sun\, Jan 22\, 2012 at 11​:42 PM\, Craig A. Berry \craig\.a\.berry@&#8203;gmail\.com wrote​:

Not that everything needing attention has gotten as much as it needs. Please do have a whack at regen/reentrl.pl and post patches about what's out-of-date or broken. The most salient comment from that file seems to be​:

/* If compiling for a threaded perl\, we will macro-wrap the system/library * interfaces (e.g. getpwent()) which have threaded versions * (e.g. getpwent_r())\, which will handle things correctly for * the Perl interpreter\, but otherwise (for XS) the wrapping does * not take place. See L\<perlxs/Thread-aware system interfaces>. */

I didn't know about that! I find the way this is done extremely confusing and perverse. First we define a set of portability wrappers (e.g. PerlSock_gethostbyaddr) that define to their POSIX equivalent (e.g. gethostbyaddr)\, and then underneath that we redefine the POSIX functions to different (and better) POSIX functions (e.g. gethostbyaddr_r). This makes no sense at all to me. It seems to me a much more sane way of dealing with this is skipping the middle stage and defining Perl{Sock\,Net\,Host\,Whatever}_* directly in the appropriate foo_r call.

Not to mention the fact that it's annoying you can't use them when writing extensions (I'd want those wrappers too\, but not under the POSIX names!). This is begging for a cleanup.

My impression (based on kinda sorta paying attention to people who actually know things) is that thread safety in the core is most likely to run aground on the combination of threads\, signals\, and the stack manipulations via which the VM does its thing.

Signals and threads are a complicated combination indeed\, but generally it's more useless than insane.

But you probably know at least some of this and I've probably missed at least some of the point of your bug report\, so please elaborate.

I think I am the one who missed something here.

Leon

p5pRT commented 12 years ago

From @doy

So does this need to stay open? If it does\, we should probably at least change the title.

-doy

p5pRT commented 12 years ago

From @Leont

On Tue\, Jul 3\, 2012 at 7​:41 PM\, Jesse Luehrs via RT \perlbug\-followup@&#8203;perl\.org wrote​:

So does this need to stay open? If it does\, we should probably at least change the title.

Not sure. I was mistaken in my OP\, but fact is that the code is currently extremely confusing (and somewhat evil). I'd go for a change of title but could also open a new ticket instead.

Leon

p5pRT commented 12 years ago

From @doy

On Wed\, Jul 04\, 2012 at 12​:24​:53PM +0300\, Leon Timmermans wrote​:

On Tue\, Jul 3\, 2012 at 7​:41 PM\, Jesse Luehrs via RT \perlbug\-followup@&#8203;perl\.org wrote​:

So does this need to stay open? If it does\, we should probably at least change the title.

Not sure. I was mistaken in my OP\, but fact is that the code is currently extremely confusing (and somewhat evil). I'd go for a change of title but could also open a new ticket instead.

Alright\, I just changed the title for now.

-doy