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

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

Open
p5pRT opened this issue Jan 22, 2012 · 7 comments
Open

Comments

@p5pRT
Copy link

p5pRT commented Jan 22, 2012

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

Searchable as RT108762$

@p5pRT
Copy link
Author

p5pRT commented Jan 22, 2012

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
Copy link
Author

p5pRT commented Jan 22, 2012

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.

<snip>

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
Copy link
Author

p5pRT commented Jan 22, 2012

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

@p5pRT
Copy link
Author

p5pRT commented Jan 22, 2012

From @Leont

On Sun, Jan 22, 2012 at 11​:42 PM, Craig A. Berry
<craig.a.berry@​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
Copy link
Author

p5pRT commented Jul 3, 2012

From @doy

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

-doy

@p5pRT
Copy link
Author

p5pRT commented Jul 4, 2012

From @Leont

On Tue, Jul 3, 2012 at 7​:41 PM, Jesse Luehrs via RT
<perlbug-followup@​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
Copy link
Author

p5pRT commented Jul 4, 2012

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@​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

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