From wayne.kohler@gmail.com

Created by wayne.kohler@gmail.com

This is a bug report for perl from wayne.kohler@​gmail.com,
generated with the help of perlbug 1.39 running under perl 5.14.0.

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

I have an piece of code where I need to open a socket connection to an xinetd based service
running on several remote hosts. Each socket connection is opened in a separate thread.

The gist of the code that creates the threads is basically as follows​:

foreach my $host (@​{$self->{hosts}}) {
  $threads{$host} = threads->new(\&get_data, $host);
  # etc
}

The thread entry function is as follows​:

sub get_data {
  my $host = shift;
  $SIG{'KILL'} = sub { threads->exit(); };
  my $remote = IO​::Socket​::INET->new(
  Proto => "tcp",
  PeerAddr => "$host",
  PeerPort => "10515",
  ) or die "cannot connect to sysmond port at $host";
  my @​results = <$remote>;
  shutdown($remote,2);
  eval join("", @​results);
}

The problem I was seeing is that the code intermittently was not returning results
for random hosts. So, I added some debug statements, and ran the application in a
loop until I saw a failure​:

sub get_data {
  my $host = shift;
  $SIG{'KILL'} = sub { threads->exit(); };
  my $remote = IO​::Socket​::INET->new(
  Proto => "tcp",
  PeerAddr => "$host",
  PeerPort => "10515",
  ) or die "cannot connect to sysmond port at $host";
  my @​results = <$remote>;
  if ($DEBUG) {
  my $sockhost = $remote->sockhost();
  my $sockport = $remote->sockport();
  my $peerport = $remote->peerport();
  my $peerhost = $remote->peerhost();
  open(FILE,">/tmp/results_$host");
  print FILE "#### Socket handle = $remote\n";
  print FILE "#### Socket host = $sockhost\n";
  print FILE "#### Socket port = $sockport\n";
  print FILE "#### Peer port = $peerport\n";
  print FILE "#### Peer host = $peerhost\n";
  print FILE "#### host = $host\n";
  print FILE join("", @​results);
  close(FILE);
  }
  shutdown($remote,2);
  eval join("", @​results);
}

When I examined the header of the debug file for the failing host, I saw the following​:

#### Socket handle = IO​::Socket​::INET=GLOB(0x6e29b20)
#### Socket host = 192.168.170.1
#### Socket port = 36888
#### Peer port = 10515
#### Peer host = 192.168.170.180
#### host = dist0

I noticed that the Peer host IP address was incorrect, it was the address of a different
host. When I examined the header of the debug file for that host, I saw the following​:

#### Socket handle = IO​::Socket​::INET=GLOB(0x6b03190)
#### Socket host = 192.168.170.1
#### Socket port = 36887
#### Peer port = 10515
#### Peer host = 192.168.170.180
#### host = ds0

So, even though I called IO​::Socket​::INET in two separate threads with different host
names, it returned the SAME IP address for the two different hosts.

I started digging into the INET.pm code, and found that it is using the following code
to perform the name to IP translation​:

sub _get_addr {
  my($sock,$addr_str, $multi) = @​_;
  my @​addr;
  if ($multi && $add{r}_{s}tr!{/}^{\text{d}}+(?:\text{.}\text{d}+)3$/) {
  (undef, undef, undef, undef, @​addr) = gethostbyname($addr_str);
  } else {
  my $h = inet_aton($addr_str);
  push(@​addr, $h) if defined $h;
  }
  @​addr;
}

Since $multi is not set, the else clause gets exectued. So, looking deeper, I found
the following XS template for inet_aton in Socket.xs​:

void
inet_aton(host)
  char * host
  CODE​:
  {
  struct in_addr ip_address;
  struct hostent * phe;

  if ((*host != '\0') && inet_aton(host, &ip_address)) {
  ST(0) = newSVpvn_flags((char *)&ip_address, sizeof ip_address, SVs_TEMP);
  XSRETURN(1);
  }

  phe = gethostbyname(host);
  if (phe && phe->h_addrtype == AF_INET && phe->h_length == 4) {
  ST(0) = newSVpvn_flags((char *)phe->h_addr, phe->h_length, SVs_TEMP);
  XSRETURN(1);
  }

  XSRETURN_UNDEF;
  }

This is using the standard gethostbyname function, which is apparantly NOT thread safe based
on some quick Google searches​:

(Excerpted from​: http​://compute.cnr.berkeley.edu/cgi-bin/man-cgi?gethostbyname+3)

Reentrant Interfaces
  The gethostbyname(), gethostbyaddr(), and gethostent() func-
  tions use static storage that is reused in each call, making
  these functions unsafe for use in multithreaded applica-
  tions.

(Excerpted from​: http​://pubs.opengroup.org/onlinepubs/009695399/functions/gethostbyname.html)

The gethostbyaddr() function need not be reentrant. A function that is not required to be reentrant is not required to be thread-safe.

(Excerpted from​: https://bugzilla.redhat.com/show_bug.cgi?id=485682)

This turns out to be much worse than it sounds​:
gethostbyname() isn't a thread safe API and could cause (and may have been
observed to cause) errors when multiple threads do gethostbyname() at the same
time.

NOTE​:

An interesting point to note is that I am running on virtual machines, hosted on
HP Blade servers with a SAN datastore. When I run this test on a system comprised
of separate physical hosts, I do not see the failure. I can make the symptom go
away on the virtual system by adding a 1 second sleep after the creation of each
thread. Perhaps on the physical system, there is enough of an iherenet delay for
whatever reason between the creation of each thread such that the problem does not
manifest itself.

The workaround I'm going to pursue is to use the IP address instead of the host
name as the PeerAddr argument to the IO​::Socket​::INET constructor.

Flags:
    category=library
    severity=high

Site configuration information for perl 5.14.0:

Configured by e3 at Tue Aug  9 15:32:56 UTC 2011.

Summary of my perl5 (revision 5 version 14 subversion 0) configuration:
   
  Platform:
    osname=linux, osvers=2.6.18-238.12.1.el5, archname=x86_64-linux-thread-multi
    uname='linux rhel5-740-slave01.vm.skarven.net 2.6.18-238.12.1.el5 #1 smp sat may 7 20:18:50 edt 2011 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Dprefix=/opt/skarven/perl5.14.0 -Dusethreads=define -Duseithreads=define -Dusemultiplicity=define'
    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.1.2 20080704 (Red Hat 4.1.2-51)', 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 /usr/lib /lib64 /usr/lib64 /usr/local/lib64
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.5.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.5'
  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.0:
    /opt/skarven/perl5.14.0/lib/site_perl/5.14.0/x86_64-linux-thread-multi
    /opt/skarven/perl5.14.0/lib/site_perl/5.14.0
    /opt/skarven/perl5.14.0/lib/5.14.0/x86_64-linux-thread-multi
    /opt/skarven/perl5.14.0/lib/5.14.0
    .


Environment for perl 5.14.0:
    HOME=/home/e3
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_ALL=POSIX
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/opt/skarven/perl5.14.0/bin:/usr/pgsql-9.0/bin:/opt/skarven/sbin:/home/e3/sbin:/home/e3/bin:/opt/skarven/bin:/usr/kerberos/bin:/usr/local/bin:/bin:/usr/bin:/usr/local/bin:/home/e3/bin:/home/e3/sbin:/home/e3/bin:/home/e3/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

From @jkeenan

On Fri Aug 26 13​:40​:24 2011, wkohler wrote​:

This is a bug report for perl from wayne.kohler@​gmail.com,
generated with the help of perlbug 1.39 running under perl 5.14.0.

Trying to boil down the original post ...

So, even though I called IO​::Socket​::INET in two separate threads with
different host
names, it returned the SAME IP address for the two different hosts.

I started digging into the INET.pm code, and found that it is using
the following code
to perform the name to IP translation​:

sub _get_addr {
my($sock,$addr_str, $multi) = @​_;
my @​addr;
if ($multi && $add{r}_{s}tr!{/}^{\text{d}}+(?:\text{.}\text{d}+)3$/) {
(undef, undef, undef, undef, @​addr) =
gethostbyname($addr_str);
} else {
my $h = inet_aton($addr_str);
push(@​addr, $h) if defined $h;
}
@​addr;
}

Since $multi is not set, the else clause gets exectued. So, looking
deeper, I found
the following XS template for inet_aton in Socket.xs​:

void
inet_aton(host)
char * host
CODE​:
{
struct in_addr ip_address;
struct hostent * phe;

    if \(\(\*host \!= '\\0'\) && inet\_aton\(host\, &ip\_address\)\) \{
            ST\(0\) = newSVpvn\_flags\(\(char \*\)&ip\_address\, sizeof

ip_address, SVs_TEMP);
XSRETURN(1);
}

    phe = gethostbyname\(host\);
    if \(phe && phe\->h\_addrtype == AF\_INET && phe\->h\_length == 4\) \{
            ST\(0\) = newSVpvn\_flags\(\(char \*\)phe\->h\_addr\, phe\-

h_length, SVs_TEMP);
XSRETURN(1);
}

    XSRETURN\_UNDEF;
    \}

This is using the standard gethostbyname function, which is apparantly
NOT thread safe based
on some quick Google searches​:

[snip one link that didn't work]

(Excerpted from​:

http​://pubs.opengroup.org/onlinepubs/009695399/functions/gethostbyname.html)

The gethostbyaddr() function need not be reentrant. A function that is
not required to be reentrant is not required to be thread-safe.

Can some familiar with threading, reentrance and socket issues evaluate
this ticket?

Thank you very much.
Jim Keenan

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

From @rgarcia

On 24 March 2012 02​:44, James E Keenan via RT <perlbug-followup@​perl.org> wrote​:
[...]

Since $multi is not set, the else clause gets exectued.  So, looking
   deeper, I found
the following XS template for inet_aton in Socket.xs​:

void
inet_aton(host)
        char *  host
        CODE​:
        {
        struct in_addr ip_address;
        struct hostent * phe;

        if ((*host != '\0') && inet_aton(host, &ip_address)) {
                ST(0) = newSVpvn_flags((char *)&ip_address, sizeof
   ip_address, SVs_TEMP);
                XSRETURN(1);
        }

        phe = gethostbyname(host);
        if (phe && phe->h_addrtype == AF_INET && phe->h_length == 4) {
                ST(0) = newSVpvn_flags((char *)phe->h_addr, phe-
   >h_length, SVs_TEMP);
                XSRETURN(1);
        }

        XSRETURN_UNDEF;
        }

This is using the standard gethostbyname function, which is apparantly
   NOT thread safe based
on some quick Google searches​:

[snip one link that didn't work]

(Excerpted from​:

http​://pubs.opengroup.org/onlinepubs/009695399/functions/gethostbyname.html)

The gethostbyaddr() function need not be reentrant. A function that is
   not required to be reentrant is not required to be thread-safe.

Can some familiar with threading, reentrance and socket issues evaluate
this ticket?

I believe that Socket.xs should use the gethostbyname_r variant if
found by Configure.

From @Leont

On Sat, Mar 24, 2012 at 2​:44 AM, James E Keenan via RT
<perlbug-followup@​perl.org> wrote​:

Can some familiar with threading, reentrance and socket issues evaluate
this ticket?

Core is very confusing in this regard. Perl #defines almost all
non-reentrant functions for core and core-extentions (that is,
anything with PERL_EXT defined), though currently I think only
List-Scalar-Util falls in that category (unless some kind of magic is
at hand).

I *really* dislike the current system. It makes thread-safe code in
core look like it isn't thread safe (I even filed a bug for this,
until I was corrected), but at the same time it doesn't offer a proper
way to enable extentions to make use of those thread-safe functions
(unless you defined PERL_EXT, which opens another bag of issues).
Worse yet, it's a bunch of all-lowercased macros that can collide with
user code. This whole design is fundamentally broken if you ask me.

I would really like this to be changed. In particular by
PerlSock_getaddrbyname and friends always be thread-safe, and
getaddrbyname not be touched.

Leon