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

Bleadperl v5.13.8-391-g7fe50b8 breaks MNOONING/Net-Daemon/Net-Daemon-0.43.tar.gz #11121

Closed
p5pRT opened this issue Feb 8, 2011 · 9 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Feb 8, 2011

Migrated from rt.perl.org#83646 (status was 'resolved')

Searchable as RT83646$

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2011

From @andk

git bisect​:


  7fe50b8 is the first bad commit
  commit 7fe50b8
  Author​: Leon Timmermans <fawaka@​gmail.com>
  Date​: Tue Jan 18 16​:40​:07 2011 +0100

  Also unblock signal handlers throwing an exception

  Also handle and test the edge case of a signal handler throwing an
  exception

symptoms​:


  The test forkm.t now hangs. Note that N​:D tests with threaded perls
  are broken for several years but there's a patch in RT ticket #55512
  and this is a completely separate issue.

perl -V​:


  Summary of my perl5 (revision 5 version 13 subversion 8) configuration​:
  Commit id​: 7fe50b8
  Platform​:
  osname=linux, osvers=2.6.32-5-xen-amd64, archname=x86_64-linux-ld
  uname='linux k81 2.6.32-5-xen-amd64 #1 smp wed jan 12 05​:46​:49 utc 2011 x86_64 gnulinux '
  config_args='-Dprefix=/home/src/perl/repoperls/installed-perls/perl/v5.13.8-391-g7fe50b8 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Uuseithreads -Duselongdouble'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=define
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.4.5', 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='long double', nvsize=16, Off_t='off_t', lseeksize=8
  alignbytes=16, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=/lib/libc-2.11.2.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.11.2'
  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'

  Characteristics of this binary (from libperl)​:
  Compile-time options​: PERL_DONT_CREATE_GVSV PERL_MALLOC_WRAP PERL_USE_DEVEL
  USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES
  USE_LONG_DOUBLE USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Feb 6 2011 22​:13​:21
  @​INC​:
  /home/src/perl/repoperls/installed-perls/perl/v5.13.8-391-g7fe50b8/lib/site_perl/5.13.8/x86_64-linux-ld
  /home/src/perl/repoperls/installed-perls/perl/v5.13.8-391-g7fe50b8/lib/site_perl/5.13.8
  /home/src/perl/repoperls/installed-perls/perl/v5.13.8-391-g7fe50b8/lib/5.13.8/x86_64-linux-ld
  /home/src/perl/repoperls/installed-perls/perl/v5.13.8-391-g7fe50b8/lib/5.13.8
  .

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Feb 14, 2011

From @Leont

On Tue, Feb 8, 2011 at 7​:01 AM, Andreas J. Koenig via RT
<perlbug-followup@​perl.org> wrote​:

# New Ticket Created by  (Andreas J. Koenig)
# Please include the string​:  [perl #83646]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=83646 >

That module is a convoluted mess that already fails its unit tests
half of the time[1], even statistical analysis[2] doesn't really
clarify things all that much. I'm tempted to blame the module's
fickleness.

Leon

[1] http​://matrix.cpantesters.org/?dist=Net-Daemon+0.43
[2] http​://analysis.cpantesters.org/solved?distv=Net-Daemon-0.43

@p5pRT
Copy link
Author

p5pRT commented Feb 14, 2011

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

@p5pRT
Copy link
Author

p5pRT commented Feb 14, 2011

From @andk

On Mon, 14 Feb 2011 04​:00​:16 -0800, "Leon Timmermans via RT" <perlbug-followup@​perl.org> said​:

  > On Tue, Feb 8, 2011 at 7​:01 AM, Andreas J. Koenig via RT
  > <perlbug-followup@​perl.org> wrote​:

# New Ticket Created by  (Andreas J. Koenig)
# Please include the string​:  [perl #83646]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=83646 >

  > That module is a convoluted mess that already fails its unit tests
  > half of the time[1], even statistical analysis[2] doesn't really
  > clarify things all that much. I'm tempted to blame the module's
  > fickleness.

What you say is unfair.

That the matrix at [1] can't show you much is due to the fact that (I'm
citing from this ticket)​:

  N​:D tests with threaded perls are broken for several years but
  there's a patch in RT ticket #55512 and this is a completely
  separate issue.

The statistical analysis at [2] cannot deal with two issues at once.
That's why I write these tickets before too much of CPAN burns.

  > [1] http​://matrix.cpantesters.org/?dist=Net-Daemon+0.43
  > [2] http​://analysis.cpantesters.org/solved?distv=Net-Daemon-0.43

As a matter of fact, statistical analysis before v5.13.8-391-g7fe50b8
was giving quite a clear result that -Dusethreads does not work. And
combine this with the fact that the test forkm.t starts to hang on non
threaded perls exactly since v5.13.8-391-g7fe50b8.

You may argue about the quality of the module but what you're saying
here is ignoring the facts.

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Feb 15, 2011

From @Leont

On Mon, Feb 14, 2011 at 8​:50 PM, Andreas J. Koenig
<andreas.koenig.7os6VVqR@​franz.ak.mind.de> wrote​:

What you say is unfair.

With «I'm tempted to blame the module's fickleness.» I didn't mean
«screw him», but more something like «the code is just plain wrong in
so many ways I'm surprised it works at all».

Anyway, it seems I've figured this out. The SIGCHLD handler calls exit
when all clients have died, but forgets about the server. It just
happened to work because the program never received any SIGCHLD after
all clients were waited for. Previously, the SIGCHLD was never
unblocked since the handler never returns, now it is unblocked as part
of stack-unwinding. Unfortunately, the SIGCHLD handler was really
badly written (if not outright crazy) and an unexpected SIGCHLD
(probably from a child) is enough to get it into a blocking wait +
infinite loop because it insists on receiving signals from dead
clients while there's only a living server.

The module is at fault. If he had read perlipc before writing signal
handlers he would have known how to write a signal handler. See the
patch for a simple solution.

As a matter of fact, statistical analysis before v5.13.8-391-g7fe50b8
was giving quite a clear result that -Dusethreads does not work.

Not quite, -Dusethreads does work in a minority of cases (and the
other way around it sometimes doesn't work on threadless perls, though
I guess that's noise). Hence the R^2 is 0.552​: significant but not the
entire story. There are other factors playing an similarly big role in
this. I have no idea what they are though. The module is fickly IMO.

Leon

@p5pRT
Copy link
Author

p5pRT commented Feb 15, 2011

From @Leont

waitpid.patch
diff --git a/t/forkm.t b/t/forkm.t
index 8dde319..03e4547 100644
--- a/t/forkm.t
+++ b/t/forkm.t
@@ -8,6 +8,7 @@ use Config ();
 use Net::Daemon::Test ();
 use Fcntl ();
 use Config ();
+use POSIX qw/WNOHANG/;
 
 
 my $debug = 0;
@@ -130,7 +131,8 @@ my %childs;
 sub CatchChild {
     &log("CatchChild: ->");
     for(;;) {
-	my $pid = wait;
+	my $pid = waitpid -1, WNOHANG;
+	last if $pid <= 0;
 	if ($pid > 0) {
 	    &log("CatchChild: $pid");
 	    if (exists $childs{$pid}) {

@p5pRT
Copy link
Author

p5pRT commented Feb 16, 2011

From @andk

On Tue, 15 Feb 2011 02​:48​:36 +0100, Leon Timmermans <fawaka@​gmail.com> said​:

  > On Mon, Feb 14, 2011 at 8​:50 PM, Andreas J. Koenig
  > <andreas.koenig.7os6VVqR@​franz.ak.mind.de> wrote​:

What you say is unfair.

  > With «I'm tempted to blame the module's fickleness.» I didn't mean
  > «screw him», but more something like «the code is just plain wrong in
  > so many ways I'm surprised it works at all».

OK, you did not say that.

  > Anyway, it seems I've figured this out. The SIGCHLD handler calls exit
  > when all clients have died, but forgets about the server. It just
  > happened to work because the program never received any SIGCHLD after
  > all clients were waited for. Previously, the SIGCHLD was never
  > unblocked since the handler never returns, now it is unblocked as part
  > of stack-unwinding. Unfortunately, the SIGCHLD handler was really
  > badly written (if not outright crazy) and an unexpected SIGCHLD
  > (probably from a child) is enough to get it into a blocking wait +
  > infinite loop because it insists on receiving signals from dead
  > clients while there's only a living server.

  > The module is at fault. If he had read perlipc before writing signal
  > handlers he would have known how to write a signal handler. See the
  > patch for a simple solution.

Thank you for figuring that out.

As a matter of fact, statistical analysis before v5.13.8-391-g7fe50b8
was giving quite a clear result that -Dusethreads does not work.

  > Not quite, -Dusethreads does work in a minority of cases (and the
  > other way around it sometimes doesn't work on threadless perls, though
  > I guess that's noise). Hence the R^2 is 0.552​: significant but not the
  > entire story. There are other factors playing an similarly big role in
  > this. I have no idea what they are though. The module is fickly IMO.

I admit it's not obvious which factors influence the results but looking
a bit closer I find that in the 5.8.[1-8] time Net​::Daemon was mostly
working under -Duseithreads. It started to fail drastically somewhere in
the 5.9.5 era with 5.11 and 5.13 *reliably* failing under -Duseithreads.
I understand that it is tempting to blame the module but it is as
tempting to blame perl because perl has changed and Net-Daemon not.

The real pity is that the maintainer is AWOL.

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Mar 9, 2011

From m.nooning@comcast.net

Thanks for the patch. I just uploaded Net-Daemon-0.48.tar.gz to CPAN.
It should be visible by tomorrow. I could only test it with Perl 5.12
on Windows and Fedora

@p5pRT
Copy link
Author

p5pRT commented Mar 14, 2011

@cpansprout - Status changed from 'open' to 'resolved'

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

1 participant