Perl / perl5

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

Locale patch breaks dist/threads/t/kill.t by triggering attribute.pm XS version mismatch errors. #20155

Closed demerphq closed 2 years ago

demerphq commented 2 years ago

Description Bisect tells me that ever since a7ff7aca38 I have been seeing test failures from dist/threads/t/kill.t:

attributes object version 0.35 does not match $attributes::VERSION 0 at lib/XSLoader.pm line 112. Compilation failed in require at lib/Thread/Queue.pm line 19. BEGIN failed--compilation aborted at lib/Thread/Queue.pm line 19. Compilation failed in require at dist/threads/t/kill.t line 31. BEGIN failed--compilation aborted at dist/threads/t/kill.t line 36.

This is the patch details:

git log -1 --stat a7ff7aca3826600e5832ffe161fe075182f19a82
commit a7ff7aca3826600e5832ffe161fe075182f19a82
Author: Karl Williamson <khw@cpan.org>
Date:   Thu Mar 11 12:24:56 2021 -0700

    locale.c: Use setlocale() for init, not P2008

    We have found bugs in the POSIX 2008 libc implementations on various
    platforms.  This code, which does the initialization of locale handling
    has always been very conservative, expecting possible failures due to
    bugs in it or the libc implementations, and backing out if necessary to
    a crippled, but workable state, if something goes wrong.

    I think we should use the oldest, most stable locale implementation in
    these circumstances

 locale.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

FWIW, I looked at the patch but beyond maybe an off by one error overwriting memory somewhere it is not clear to me why this patch would lead to this error. It is also not clear why it would fail for me and not in CI. I would be happy to provide any additional details required to debug.

Steps to Reproduce

$ git reset --hard a7ff7aca3826600e5832ffe161fe075182f19a82
HEAD is now at a7ff7aca38 locale.c: Use setlocale() for init, not P2008

$ make -j8 test_prep > /tmp/test_prep.log 2>&1; ./perl -Ilib dist/threads/t/kill.t
attributes object version 0.35 does not match $attributes::VERSION 0 at lib/XSLoader.pm line 112.
Compilation failed in require at lib/Thread/Queue.pm line 19.
BEGIN failed--compilation aborted at lib/Thread/Queue.pm line 19.
Compilation failed in require at dist/threads/t/kill.t line 31.
BEGIN failed--compilation aborted at dist/threads/t/kill.t line 36.

$ git reset --hard a7ff7aca3826600e5832ffe161fe075182f19a82^
HEAD is now at 50ffb02bd2 locale.c: Refactor some derived #defines

$ make -j8 test_prep > /tmp/test_prep.log 2>&1; ./perl -Ilib dist/threads/t/kill.t
1..18
ok 1 - Loaded
ok 2 - Created thread
ok 3 - Thread sleeping
ok 4 - Signalled thread
ok 5 - Thread received signal
ok 6 - No thread return value
ok 7 - Thread termination warning
ok 8 - Semaphore created
ok 9 - Created thread
ok 10 - Thread received semaphore
ok 11 - Suspended thread
ok 12 - Thread suspending
ok 13 - Thread resuming
ok 14 - Signalled thread to terminate
ok 15 - Thread caught termination signal
ok 16 - Thread done
ok 17 - Thread return value
ok 18 - Ignore signal to terminated thread

Expected behavior The second output, kill.t should not fail.

Perl configuration

./Configure -Dusethreads -Doptimize=-g -d -Dusedevel -Dcc=ccache\ gcc -Dld=gcc -DDEBUGGING
./perl -Ilib -V
Summary of my perl5 (revision 5 version 37 subversion 3) configuration:
  Commit id: 50ffb02bd2b94a3e1ca31fc74825149050e13940
  Platform:
    osname=linux
    osvers=5.14.0-1049-oem
    archname=x86_64-linux-thread-multi
    uname='linux oncidium 5.14.0-1049-oem #56-ubuntu smp fri aug 12 10:23:08 utc 2022 x86_64 x86_64 x86_64 gnulinux '
    config_args='-Dusethreads -Doptimize=-g -d -Dusedevel -Dcc=ccache gcc -Dld=gcc -DDEBUGGING'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='gcc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='9.4.0'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='gcc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /usr/lib64
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.31.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.31'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl): 
  Compile-time options:
    DEBUGGING
    HAS_TIMES
    MULTIPLICITY
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_HASH_FUNC_SIPHASH13
    PERL_HASH_USE_SBOX32
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_TRACK_MEMPOOL
    PERL_USE_DEVEL
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_ITHREADS
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_REENTRANT_API
    USE_THREAD_SAFE_LOCALE
  Built under linux
  Compiled at Aug 25 2022 15:09:49
  %ENV:
    PERLBREW_CONFIGURE_FLAGS="-de -Dcc=ccache\ gcc -Dld=gcc"
    PERLBREW_HOME="/home/yorton/.perlbrew"
    PERLBREW_MANPATH="/home/yorton/perl5/perlbrew/perls/perl-5.34.1/man"
    PERLBREW_PATH="/home/yorton/perl5/perlbrew/bin:/home/yorton/perl5/perlbrew/perls/perl-5.34.1/bin"
    PERLBREW_PERL="perl-5.34.1"
    PERLBREW_ROOT="/home/yorton/perl5/perlbrew"
    PERLBREW_SHELLRC_VERSION="0.88"
    PERLBREW_VERSION="0.88"
  @INC:
    lib
    /usr/local/lib/perl5/site_perl/5.37.3/x86_64-linux-thread-multi
    /usr/local/lib/perl5/site_perl/5.37.3
    /usr/local/lib/perl5/5.37.3/x86_64-linux-thread-multi
    /usr/local/lib/perl5/5.37.3
jkeenan commented 2 years ago

Where I customarily test blead I am unable to reproduce these failures.

$ uname -mrs;gcc --version | head -1; gitcurr; git describe;./perl -Ilib -V:config_args
FreeBSD 12.3-RELEASE-p1 amd64
gcc (FreeBSD Ports Collection) 10.3.0
blead
v5.37.2-338-gc74a928a43
config_args='-des -Dusedevel -Duseithreads -DDEBUGGING -Doptimize=-g -Dcc=gcc -Dld=gcc';

$ cd t;TEST_JOBS=1 ./perl harness ../dist/threads/t/*.t; cd -
...
All tests successful.
Files=25, Tests=591, 39 wallclock secs ( 0.16 usr  0.05 sys +  6.53 cusr  0.42 csys =  7.16 CPU)
Result: PASS
demerphq commented 2 years ago

@jkeenan notice your Configure args are different.

./Configure -Dusethreads -Doptimize=-g -d -Dusedevel -DDEBUGGING

notice I have "usethreads" not "useithreads".

Tux commented 2 years ago

Unless you explicitly specify both, they are inherently the same: the default for usethreads is useithreads and useithreads is set to usethreads after its specification is done, so it should not matter

demerphq commented 2 years ago

@Tux ok, thanks.

@khwilliamson I have narrowed this down a bit. If I add use attributes; to threads::shared the problem goes away. I believe that somehow the a7ff7aca38 patch meant that something changed with how attributes are loaded, so that attributes.pm does not get loaded before it is used. I also stepped through the code in gdb to be sure. It is not clear to me if in the older code it loaded attributes as a side-effect and now it no longer does and somehow this meant nobody noticed that threads::shared is actually broken in not explicitly using attributes, or if it is because somehow module loading logic has been broken by this change. Given it only affects this single test im inclined to think you removed a side-effect that was hiding this bug.

jkeenan commented 2 years ago

@jkeenan notice your Configure args are different.

./Configure -Dusethreads -Doptimize=-g -d -Dusedevel -DDEBUGGING

notice I have "usethreads" not "useithreads".

No difference here.

$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Dusethreads -DDEBUGGING -Doptimize=-g -Dcc=gcc -Dld=gcc';
[perlmonger: perl] $ cd t; TEST_JOBS=1 ./perl harness ../dist/threads/t/kill*.t; cd -
../dist/threads/t/kill.t ... ok     
../dist/threads/t/kill2.t .. ok   
../dist/threads/t/kill3.t .. ok   
All tests successful.
Files=3, Tests=24,  9 wallclock secs ( 0.02 usr  0.01 sys +  0.29 cusr  0.07 csys =  0.38 CPU)
Result: PASS
demerphq commented 2 years ago

FWIW, it seems that https://github.com/Perl/perl5/pull/20157 fixes the issue, although it is still not clear to me if it should be required or why your patch exposed this issue this way. It does seem plausible that the old locale code loaded attributes.pm implicitly and stuff just worked that now doesn't. But why it only fails here and not elsewhere, and etc, are all open questions.

bram-perl commented 2 years ago

Number parsing fails after a thread was created.. So it's a lot more generic then just attributes.pm failing

A reduced test case (which could be used as a starting point for an extra test):

use strict;
use warnings;
use Test::More;

BEGIN {
    use Config;
    if (! $Config{'useithreads'}) {
        plan skip_all => "Perl not compiled with 'useithreads'";
    }
}

use threads;

my $x = eval "1.25";
is("$x", "1.25", "number is ok before thread");
my $str_x = "$x";

my $thr = threads->create(sub {});
$thr->join();

is("$x", "$str_x", "number stringifies the same after thread");

my $y = eval "1.25";
is("$y", "1.25", "number is ok after threads");
is("$y", "$str_x", "new number stringifies the same as old number");

done_testing();

Running it:

$ LC_ALL= LC_NUMERIC=nl_NL ./perl -Ilib t-20155.pl
ok 1 - number is ok before thread
not ok 2 - number stringifies the same after thread
#   Failed test 'number stringifies the same after thread'
#   at t-20155.pl line 23.
#          got: '1,25'
#     expected: '1.25'
not ok 3 - number is ok after threads
#   Failed test 'number is ok after threads'
#   at t-20155.pl line 26.
#          got: '1'
#     expected: '1.25'
not ok 4 - new number stringifies the same as old number
#   Failed test 'new number stringifies the same as old number'
#   at t-20155.pl line 27.
#          got: '1'
#     expected: '1.25'
1..4
# Looks like you failed 3 tests of 4.

and

$ LC_ALL= LC_NUMERIC=en_US ./perl -Ilib t-20155.pl
ok 1 - number is ok before thread
ok 2 - number stringifies the same after thread
ok 3 - number is ok after threads
ok 4 - new number stringifies the same as old number
1..4

For those unaware:

bram-perl commented 2 years ago

One final observation: adding a use locale; alters the behavior and causes the number to be correctly parsed

use threads;

my $x = eval "1.25";
print "Before thread: $x\n";

my $thr = threads->create(sub {});
$thr->join();

print "After thread: $x\n";

{
        my $y = eval "1.25";
        print "New number (outside `use locale`-block): $y\n";
}
{
        use locale;
        my $y = eval "1.25";
        print "New number (inside `use locale`-block): $y\n";
}

Running:

$ LC_ALL= LC_NUMERIC=nl_NL ./perl -Ilib ../t2.pl
Before thread: 1.25
After thread: 1,25
New number (outside `use locale`-block): 1
New number (inside `use locale`-block): 1,25
khwilliamson commented 2 years ago

This is a bug only in boxes running Posix 2008 threaded locales, and has to do with the thread not being switched soon enough from the global locale. I had commits already in my pipeline to fix this; I'll try to move them up.

In the meantime, just don't set LC_NUMERIC to a non-dot radix locale in your environment at perl initiation

bram-perl commented 2 years ago

I had commits already in my pipeline to fix this; I'll try to move them up.

Do you have an estimate on that? IMO it should be fixed before 5.37.4 is released..

Right now it just - silently - fails at basic math when there was a thread (and LC_NUMERIC is set to a non-dot radix) Example:

$ cat gh-20515.pl
use strict;
use warnings;

BEGIN {
    use Config;
    if (! $Config{'useithreads'}) {
        die "Perl not compiled with 'useithreads'\n";
    }
}

use threads;

print "Before threads:\n";
do "./gh-20155-b.pl";

my $thr = threads->create(sub {});
$thr->join();

print "After threads:\n";
do "./gh-20155-b.pl";

and

$ cat gh-20155-b.pl
print "5.55 + 5.55 = " . (5.55 + 5.55) . "\n";

Running:

$ LC_ALL=  LC_NUMERIC=nl_NL.UTF-8 ./perl -Ilib gh-20155.pl
Before threads:
5.55 + 5.55 = 11.1
After threads:
5.55 + 5.55 = 10
khwilliamson commented 2 years ago

20178 fixes this, but needs more work, and advice

bram-perl commented 2 years ago

@khwilliamson release of 5.37.4 is getting closer.. have you been able to make progress on this? (/on a better fix then PR #20178 ?)