From singwang.ho08@imperial.ac.uk

Hello,

I have been using a perl code I wrote in an older perl version. After
upgrading my Ubuntu distribution (from 8.04 Hardy to 9.10 Karmic), I
have encoutered an perl error in my code. Unfortunately, I do not
remember my
version of perl in Hardy (it is >=5.8.8). My current version of perl
is v5.10.0 built for i486-linux-gnu-thread-multi.

The code was built to support older version of perl that do not support
'threads->list(threads​::joinable)'.

The code now gives me an error saying an undefined object is requested
to be
joined when threads->object($tid)->join is invoked. Strangely, if I
print
the tid to STDOUT by print("$tid\n") first, the code works fine.

I believe this is an issue to do with how 'threads->tid()' stores the
Thread
ID. The variable, $tid, is only converted to a proper string after
'print()'
is invoked. The 'Stringify' option in perl did not fix the problem
either.

Best,
Sing Wang Ho

A test case is attached here​:
!/usr/bin/perl -w

use strict;
use threads;
use threads​::shared;

my $max_threads = 10;

my $thread_created = 0;

my @​joinable_tid : shared = ();

for(my $i=0; $i < 100; $i++) {
  threads->create(\&worker); $thread_created++;
  print "Thread started $i\n";
  wait_for_available_thread($max_threads);
}
wait_for_available_thread(0);
print ("DONE");

# Thread Monitoring
sub wait_for_available_thread {
  while($thread_created > $_[0]) {
  my $createMore = 0;
  while($createMore == 0) {
  { lock(@​joinable_tid);
  if(@​joinable_tid != 0) {
  $createMore = 1;
  $thread_created -= scalar(@​joinable_tid);
  foreach my $tid (@​joinable_tid) {
#print "Joining Thread​: $tid\n"; #Strangely uncommenting this line will
make the code work
  threads->object($tid)->join();
  }
  @​joinable_tid = ();
  }
  }
  threads->yield;
  }
  sleep(1);
  }
}

# Worker Thread
sub worker {
  my $ret = 0;
  # do some work....
  { lock(@​joinable_tid);
  push(@​joinable_tid, threads->tid);
  }
}

Flags​:
  category=library
  severity=medium

Site configuration information for perl 5.10.0​:

Configured by Debian Project at Thu Oct 1 22​:38​:45 UTC 2009.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration​:
  Platform​:
  osname=linux, osvers=2.6.24-23-server,
archname=i486-linux-gnu-thread-multi
  uname='linux vernadsky 2.6.24-23-server #1 smp wed apr 1 22​:22​:14
utc 2009 i686 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN
-Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr
-Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10
-Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5
-Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local
-Dsitelib=/usr/local/share/perl/5.10.0
-Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1
-Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1
-Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl
-Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio
-Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib
-Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN
-fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing
-pipe -I/usr/local/include'
  ccversion='', gccversion='4.4.1', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
  alignbytes=4, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /usr/lib64
  libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
  perllibs=-ldl -lm -lpthread -lc -lcrypt
  libc=/lib/libc-2.10.1.so, so=so, useshrplib=true,
libperl=libperl.so.5.10.0
  gnulibc_version='2.10.1'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'

Locally applied patches​:
 

@​INC for perl 5.10.0​:
  /etc/perl
  /usr/local/lib/perl/5.10.0
  /usr/local/share/perl/5.10.0
  /usr/lib/perl5
  /usr/share/perl5
  /usr/lib/perl/5.10
  /usr/share/perl/5.10
  /usr/local/lib/site_perl
  .

Environment for perl 5.10.0​:
  HOME=/home/singwang
  LANG=en_GB.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)

PATH=/home/singwang/bin​:/usr/local/sbin​:/usr/local/bin​:/usr/sbin​:/usr/bin​:/sbin​:/bin​:/usr/games
  PERL_BADLANG (unset)
  SHELL=/bin/bash

From @iabyn

On Fri, Mar 05, 2010 at 12​:46​:56PM -0800, Sing Wang Ho wrote​:

# New Ticket Created by Sing Wang Ho
# Please include the string​: [perl #73330]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=73330 >

Hello,

I have been using a perl code I wrote in an older perl version. After
upgrading my Ubuntu distribution (from 8.04 Hardy to 9.10 Karmic), I
have encoutered an perl error in my code. Unfortunately, I do not
remember my
version of perl in Hardy (it is >=5.8.8). My current version of perl
is v5.10.0 built for i486-linux-gnu-thread-multi.

The code was built to support older version of perl that do not support
'threads->list(threads​::joinable)'.

The code now gives me an error saying an undefined object is requested
to be
joined when threads->object($tid)->join is invoked. Strangely, if I
print
the tid to STDOUT by print("$tid\n") first, the code works fine.

I think the issue is a race condition​: once the parent has called join, but
before the child is actually joined, threads->object can't find the object
(it skips ones marked as joined, and the child has been marked as joined
even though its not yet joined. I think the printing/stringification
just affects the timing and thus race condition.

It's a regression, introduced in 5.8.9, 5.9.5 and 5.10.0 and is still
present in blead.

The following short test case demonstrates the issue​:

  use threads;
  my $child = threads->create(sub {
  my $tid = threads->tid;
  sleep 2;
  my $thr = threads->object($tid);
  die "failed to get thread object from tid [$tid]\n" unless defined $thr;
  });
  #sleep 2;
  $child->join;
 
This dies; however, reverse the two sleeps (ie comment one, uncommment the
other), and it runs to completion.

The fix is probably to delay the

  thread->state |= PERL_ITHR_JOINED;

in ithread_join() in threads.xs, or introduce a further flag,
PERL_ITHR_JOINING, say; but I'd have to think about it a lot more first or
I'd probably end up just shifting the race condition about.

--
In economics, the exam questions are the same every year.
They just change the answers.

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

From @iabyn

On Sun, Mar 07, 2010 at 08​:53​:57PM +0000, Dave Mitchell wrote​:

I think the issue is a race condition​:

On further reflection, the test code below demonstrates a different bug
which I happened to unearth while trying to create a small test case; that
bug isn't relevant to the OP's code.

The OP's bug is actually that ithread_object() tests for SvOK without
doing an SvGETMAGIC() first. Since the OP's code is​:

  my @​joinable_tid : shared = ();
  ...
  foreach my $tid (@​joinable_tid) {

$tid is a fresh PVLV and is thus not SvOK

To the OP​: you can work round this bug in 5.10.0 by using "$tid" (i.e. in
double quotes) rather than a bare $tid in

  threads->object("$tid")->join();

Note also that from 5.8.9 onwards, there's an easy way to get a list of
joinable threads​:

  my @​joinable = threads->list(threads​::joinable);

The following short test case demonstrates the issue​:

use threads;
my $child = threads\->create\(sub \{
my $tid = threads\->tid;
sleep 2;
my $thr = threads\->object\($tid\);
die "failed to get thread object from tid \[$tid\]\\n" unless defined $thr;
\}\);
\#sleep 2;
$child\->join;

--
Lear​: Dost thou call me fool, boy?
Fool​: All thy other titles thou hast given away; that thou wast born with.

From @jdhedden

The OP's bug is actually that ithread_object() tests for SvOK without
doing an SvGETMAGIC() first. Since the OP's code is​:

Dave,
Is the following sufficient to fix this?

void
ithread_object(...)
  PREINIT​:
  char *classname;
+ SV *arg;
  UV tid;
  ithread *thread;
  int state;
  int have_obj = 0;
  dMY_POOL;
  CODE​:
  /* Class method only */
  if (SvROK(ST(0))) {
  Perl_croak(aTHX_ "Usage​: threads->object($tid)");
  }
  classname = (char *)SvPV_nolen(ST(0));

+ arg = ST(1);
+ SvGETMAGIC(arg);

- if ((items < 2) || ! SvOK(ST(1))) {
+ if ((items < 2) || ! SvOK(arg)) {
  XSRETURN_UNDEF;
  }

  /* threads->object($tid) */
- tid = SvUV(ST(1));
+ tid = SvUV(arg);

Also, not knowing how to generate a PVLV, is this a sufficient test
for this bug?

  threads->create(sub { sleep(1); });

  my @​tids = qw/1/;

  for my $tid (@​tids) {
  my $thr = threads->object($tid);
  ok($thr, 'Obtained threads object');
  $thr->join() if $thr;
  }

Thanks.

From @nwc10

On Sun, Mar 07, 2010 at 09​:37​:59PM +0000, Dave Mitchell wrote​:

On Sun, Mar 07, 2010 at 08​:53​:57PM +0000, Dave Mitchell wrote​:

I think the issue is a race condition​:

On further reflection, the test code below demonstrates a different bug
which I happened to unearth while trying to create a small test case; that
bug isn't relevant to the OP's code.

The OP's bug is actually that ithread_object() tests for SvOK without
doing an SvGETMAGIC() first. Since the OP's code is​:

my @​joinable\_tid : shared = \(\);
\.\.\.
foreach my $tid \(@​joinable\_tid\) \{

$tid is a fresh PVLV and is thus not SvOK

To the OP​: you can work round this bug in 5.10.0 by using "$tid" (i.e. in
double quotes) rather than a bare $tid in

threads\->object\("$tid"\)\->join\(\);

Note also that from 5.8.9 onwards, there's an easy way to get a list of
joinable threads​:

my @​joinable = threads\->list\(threads​::joinable\);

Actually, given that threads (and threads​::shared) are dual life, it's viable
to install the newer versions of the modules from CPAN, to get this back on
5.8.8 (and earlier)

[assuming that I'm not missing a special case, because this is conditionally
compiled in and only available on 5.8.9 onwards)

Nicholas Clark

From @jdhedden

Submitted patch to blead for threads v1.76 which contains fixes for the
two issues in the bug report​:

- Handle magic on arg to ->object() (bug #73330)
- Make ->object(threads->tid()) work like ->self() (bug #73330)

From singwang.ho08@imperial.ac.uk

Hello,

Using​:
threads->object("$tid")->join()
instead of​:
threads->object($tid)->join()

Is the workaround I am looking for, thanks!

I am not a perl expert but I don't think it is a race condition because
adding a sleep() infront of join() did not solve the problem.

sleep 1;
threads->object($tid)->join()

Furthermore, I used lock() to prevent other threads from using it.

Best,
Sing Wang Ho

From @iabyn

On Mon, Mar 08, 2010 at 10​:35​:01AM -0500, Jerry D. Hedden wrote​:

The OP's bug is actually that ithread_object() tests for SvOK without
doing an SvGETMAGIC() first. Since the OP's code is​:

Dave,
Is the following sufficient to fix this?

Looks okay

void
ithread_object(...)
PREINIT​:
char *classname;
+ SV *arg;
UV tid;
ithread *thread;
int state;
int have_obj = 0;
dMY_POOL;
CODE​:
/* Class method only */
if (SvROK(ST(0))) {
Perl_croak(aTHX_ "Usage​: threads->object($tid)");
}
classname = (char *)SvPV_nolen(ST(0));

+ arg = ST(1);
+ SvGETMAGIC(arg);

- if ((items < 2) || ! SvOK(ST(1))) {
+ if ((items < 2) || ! SvOK(arg)) {
XSRETURN_UNDEF;
}

    /\* threads\->object\($tid\) \*/

- tid = SvUV(ST(1));
+ tid = SvUV(arg);

Also, not knowing how to generate a PVLV, is this a sufficient test
for this bug?

threads\->create\(sub \{ sleep\(1\); \}\);

my @​tids = qw/1/;

for my $tid \(@​tids\) \{
    my $thr = threads\->object\($tid\);
    ok\($thr\, 'Obtained threads object'\);
    $thr\->join\(\) if $thr;
\}

Tied elements (or shared array elements) are PVLVs. The following is a
simple example that's not dependent on threads​::shared. The sleep is
required to get round the second bug.

  use threads;

  {
  package Mytie;
  sub TIESCALAR { bless {} }
  sub FETCH { threads->tid }
  }

  tie my $tid, Mytie;
  my $thr = threads->new( sub {
  ok(threads->object($tid), 'Obtained threads object');
  ok(threads->object("$tid"), 'Obtained threads object (stringified)');
  });
  sleep 2;
  $thr->join;

--
I don't want to achieve immortality through my work... I want to achieve
it through not dying.
  -- Woody Allen

From @iabyn

On Mon, Mar 08, 2010 at 04​:26​:48PM -0500, Jerry D. Hedden wrote​:

This upgrade fixes bug #73330 as follows​:
- Handle magic on arg to ->object() (bug #73330)
- Make ->object(threads->tid()) work like ->self() (bug #73330)

I thought I'd just mention once again how intensely I hate this sort of
patching to threads/threads​::shared.

Its one big patch that conflates multiple separate fixes, plus a whole
bunch of obscuring boiler-plate to bump the version number. Thus for
example for me to have a quick review of the magic object fix involved a
whole bunch of wasted time looking through dross and/or unrelated stuff
that I don't even know whether is relevant. There should be one patch
applied to blead for every discrete issue that's been fixed, followed
occasionally by a boiler-plate patch that bumps the version, when you want
to put out all the new fixes to CPAN.

As regards the specifics of this patch​:

* I don't see how

- Make ->object(threads->tid()) work like ->self() (bug #73330)

Appears in this bug report. The second issue I noticed is that a thread
gets marked as JOINED even is its still running, just because the parent
is waiting in a join() call.

Absent a clear discussion of the issue in RT, the patch needs a better
description of what issue it is you're fixing.

* Is it really necessary to create a new test file, object.t?

* The "Memory consumption" section​:

I don't like "On most systems, frequent and continual creation and
destruction of threads can lead to ever-increasing growth in the memory
footprint". If this is the case,then its a bug (or bugs), which we should
be working towards minimising or eliminating. If we need to say
something, then we should imply that it is bugs and not a general fact of
life.

--
This email is confidential, and now that you have read it you are legally
obliged to shoot yourself. Or shoot a lawyer, if you prefer. If you have
received this email in error, place it in its original wrapping and return
for a full refund. By opening this email, you accept that Elvis lives.

From @iabyn

On Mon, Mar 08, 2010 at 05​:54​:58PM +0000, Sing Wang Ho wrote​:

I am not a perl expert but I don't think it is a race condition because
adding a sleep() infront of join() did not solve the problem.

sleep 1;
threads->object($tid)->join()

Furthermore, I used lock() to prevent other threads from using it.

Sorry I didn't make it clear. While attempting to diagnose your issue, I
wrote a test case that demonstrates an additional bug / race condition in
perl's threads library that will need fixing, but is not an issue in your
code.

--
I don't want to achieve immortality through my work... I want to achieve
it through not dying.
  -- Woody Allen

From @jdhedden

On Tue, Mar 9, 2010 at 09​:46, Dave Mitchell <davem@​iabyn.com> wrote​:

On Mon, Mar 08, 2010 at 04​:26​:48PM -0500, Jerry D. Hedden wrote​:

This upgrade fixes bug #73330 as follows​:
- Handle magic on arg to ->object() (bug #73330)
- Make ->object(threads->tid()) work like ->self() (bug #73330)

I thought I'd just mention once again how intensely I hate this sort of
patching to threads/threads​::shared.

Its one big patch that conflates multiple separate fixes, plus a whole
bunch of obscuring boiler-plate to bump the version number. Thus for
example for me to have a quick review of the magic object fix involved a
whole bunch of wasted time looking through dross and/or unrelated stuff
that I don't even know whether is relevant. There should be one patch
applied to blead for every discrete issue that's been fixed, followed
occasionally by a boiler-plate patch that bumps the version, when you want
to put out all the new fixes to CPAN.

Understood. I've withdrawn the patch, and will resubmit accordingly.

As regards the specifics of this patch​:

* I don't see how

- Make ->object(threads->tid()) work like ->self() (bug #73330)

Appears in this bug report. The second issue I noticed is that a thread
gets marked as JOINED even is its still running, just because the parent
is waiting in a join() call.

The problem you noted was that

  my $tid = threads->tid();
  ...
  my $thr = threads->object($tid)

did not work inside the child thread if the parent thread had already done
->join() on that thread. I didn't see this as a problem with the JOINED flag;
I think the current behavior is appropriate.

I did see an inconsistency with how ->object() and ->self() work inside a
thread. ->self() always works, but ->object($tid) where $tid is the value for
the current thread can return undef if the thread is detached or joined. My
change makes it such that ->self() and ->object(threads->tid()) work the same.
This solution addresses the issue you noted.

Regarding the JOINED flag, again, I don't see anything that needs changing.
Once ->join() is called it prevents the thread from showing up in ->list() and
->object() (except as I have changed it), and I can't think of any case where
this behavior would be problematic. Do you?

Absent a clear discussion of the issue in RT, the patch needs a better
description of what issue it is you're fixing.

* Is it really necessary to create a new test file, object.t?

Sure, it could be put into an existing file. Which do you think might be best?

* The "Memory consumption" section​:

I don't like "On most systems, frequent and continual creation and
destruction of threads can lead to ever-increasing growth in the memory
footprint". If this is the case,then its a bug (or bugs), which we should
be working towards minimising or eliminating. If we need to say
something, then we should imply that it is bugs and not a general fact of
life.

I have seen several postings to p5p, and I think, also, there have been some
bug reports regarding memory and threads. The responses have always been
inconsistent​: some confirmations, some non-reproducible. My ignorant opinion
is that in all cases we are dealing with OS issues related to threads and
memory. In the case of Win32, this very much seems to be the case.

The only idea I have for why Perl might be responsible is related to memory
fragmentation.

At any rate, given the non-responsiveness of the developers to grapple with
this issue (no disparagement to anyone intended), I felt that commenting on
it in the POD would be helpful to users.

From @iabyn

On Tue, Mar 09, 2010 at 10​:34​:34AM -0500, Jerry D. Hedden wrote​:

Understood. I've withdrawn the patch, and will resubmit accordingly.

Okay thanks.
Sorry if I sounded a bit harsh yesterday. I had a headache and some
non perl-releated shite had also come up.

Note that because of the code freeze, I'm assuming these will get applied
post-5.12.

The problem you noted was that

my $tid = threads\->tid\(\);
\.\.\.
my $thr = threads\->object\($tid\)

did not work inside the child thread if the parent thread had already done
->join() on that thread. I didn't see this as a problem with the JOINED flag;
I think the current behavior is appropriate.

Ah I see. The idea of having a thread marked as 'JOINED' while its still
running made (and still makes) me twitch, but given the current limited
usage of that flag, I think your fix covers everything. It might be worth
amended the comment for PERL_ITHR_JOINED at some point to

  "Thread is being / has been joined"

* The "Memory consumption" section​:

I don't like "On most systems, frequent and continual creation and
destruction of threads can lead to ever-increasing growth in the memory
footprint". If this is the case,then its a bug (or bugs), which we should
be working towards minimising or eliminating. If we need to say
something, then we should imply that it is bugs and not a general fact of
life.

I have seen several postings to p5p, and I think, also, there have been some
bug reports regarding memory and threads. The responses have always been
inconsistent​: some confirmations, some non-reproducible. My ignorant opinion
is that in all cases we are dealing with OS issues related to threads and
memory. In the case of Win32, this very much seems to be the case.

The only idea I have for why Perl might be responsible is related to memory
fragmentation.

At any rate, given the non-responsiveness of the developers to grapple with
this issue (no disparagement to anyone intended), I felt that commenting on
it in the POD would be helpful to users.

Ok fair enough.

--
Overhead, without any fuss, the stars were going out.
  -- Arthur C Clarke

From pshaydo@elverils.com

On Tue, 9 Mar 2010 10​:34​:34 -0500
"Jerry D. Hedden" <jdhedden@​cpan.org> wrote​:

* The "Memory consumption" section​:

I don't like "On most systems, frequent and continual creation and
destruction of threads can lead to ever-increasing growth in the
memory footprint". If this is the case,then its a bug (or bugs),
which we should be working towards minimising or eliminating. If we
need to say something, then we should imply that it is bugs and not
a general fact of life.

I have seen several postings to p5p, and I think, also, there have
been some bug reports regarding memory and threads. The responses
have always been inconsistent​: some confirmations, some
non-reproducible.

http​://rt.perl.org/rt3/Public/Bug/Display.html?id=69598 is one of such
bug reports.

My ignorant opinion is that in all cases we are
dealing with OS issues related to threads and memory. In the case of
Win32, this very much seems to be the case.

Not quite understand. I have some multithreaded programs written in C
and it's not common for them to leak memory.

The only idea I have for why Perl might be responsible is related to
memory fragmentation.

My system perl's RSS grows about 3.8M per 1000 threads, ActiveState Perl
grows about 11M per 1000 threads, this isn't changed from run to run,
it doesn't matter if I'm doing something in the thread or not. I can't
imagine how this steady growth may be caused by memory fragmentation
alone.

--
Best regards
Pavel Shaydo

From @khwilliamson

Dave Mitchell wrote​:

On Tue, Mar 09, 2010 at 10​:34​:34AM -0500, Jerry D. Hedden wrote​:

Understood. I've withdrawn the patch, and will resubmit accordingly.

Okay thanks.
Sorry if I sounded a bit harsh yesterday. I had a headache and some
non perl-releated shite had also come up.

I maintain that wording needs to be added in either perlhack or
perlrepository (actually these two should also be compared and
reconciled) that describes more what the criteria should be in a good patch.