Bug #114468 for Thread-Queue: 07_lock.t leaks temp files, and other bad things

Wed May 18 12:02:06 2016 exodist7 [...] gmail.com - Ticket created

Subject:

07_lock.t leaks temp files, and other bad things

https://metacpan.org/source/JDHEDDEN/Thread-Queue-3.09/t/07_lock.t This test detaches a thread and lets the parent thread end before the detached thread. This causes perl to skip global destruction. Without global destruction you get some nasty things like temp file and SHM leaks. Changing the test so that it does not detach, but instead eventually joins, will allow global destruction to take place and things don't leak anymore.

Wed May 18 12:46:43 2016 jdhedden [...] cpan.org - Correspondence added

On 2016-05-18 12:02:06, EXODIST wrote: Show quoted text

> This test detaches a thread and lets the parent thread end before the > detached thread.

I don't think that is the case. The detached thread locks the queue that is being tested (line 34). This lock is not released until the thread exits its subroutine at which point is will start to be destroyed. Meanwhile, the main thread is blocked from reading the queue until the lock is released (line 46). Following that it does one more yield (line 48) to allow for the thread to be destroyed before itself exiting. Do you have any evidence that indicates the main thread exits before the detached thread is destroyed? Show quoted text

> This [main thread exiting with detached threads] causes perl to skip global > destruction.

I am not aware that this is the case. Can you provide documentation/evidence regarding this?

Wed May 18 12:46:44 2016 The RT System itself - Status changed from 'new' to 'open'

Wed May 18 12:56:05 2016 exodist7 [...] gmail.com - Correspondence added

On Wed May 18 09:46:43 2016, JDHEDDEN wrote: Show quoted text

> On 2016-05-18 12:02:06, EXODIST wrote:

> > This test detaches a thread and lets the parent thread end before the > > detached thread.

> > I don't think that is the case. The detached thread locks the queue > that is being tested (line 34). This lock is not released until the > thread exits its subroutine at which point is will start to be > destroyed. > > Meanwhile, the main thread is blocked from reading the queue until the > lock is released (line 46). Following that it does one more yield > (line 48) to allow for the thread to be destroyed before itself > exiting. > > Do you have any evidence that indicates the main thread exits before > the detached thread is destroyed? >

> > This [main thread exiting with detached threads] causes perl to skip > > global > > destruction.

> > I am not aware that this is the case. Can you provide > documentation/evidence regarding this?

Add the following to your test just above the exit(0) { package foo; sub DESTROY { print "Global Destruction thread: " . threads->tid . "\n"; }; } our $foo = bless {}, 'foo'; The message will never print. However if you change the test so that you save the thread, and call ->join on it before the exit, then the message will print. Yesterday on #p5p several of us were discussing it. We noticed the problem because the perl test suite was not cleaning up all its temp files, this test is one of the culprits. Eventually we had to dive into thread code and confirm there is (intentional!!) code to block global destruction if there are running detached threads. When we change this test to join on the thread instead of detach the leak stops. Test2 (the new Test-Simple) uses temp files for threaded code, and we noticed it's destruction never occurred for this test, which is why there are temp files being left behind.

Wed May 18 12:58:43 2016 exodist7 [...] gmail.com - Correspondence added

It is important to note that the speed of the computer matters, this is racey, sometimes the detahced thread does finish first, but sometimes it does not. You locking works for making the threads both wait till the end, but at the end they both race to shut-down first.

Wed May 18 13:01:10 2016 exodist7 [...] gmail.com - Correspondence added

Here is a snippet of the detective work: <haarg> [14:45:07] 5.8 sometimes it works, sometimes it segfaults, sometimes it complains about 2 threads running at exit <haarg> [14:45:40] and the other various things that threads always do <haarg> [14:46:42] monkeypatching threads is really ugly, but as far as i can tell there isn't any other way to get the information needed <haarg> [14:47:07] so it would need some other workaround <hobbs> [14:52:25] github.com/Perl/perl5/commit/ae3fba3defe4ed89f6b20b720860824178f77f52 <hobbs> [14:52:30] https://github.com/Perl/perl5/commit/ae3fba3defe4ed89f6b20b720860824178f77f52 <dipsy> [14:52:31] [ further refinement to #29796 (cleanup veto) · Perl/perl5@ae3fba3 · GitHub ] <hobbs> [14:53:31] https://github.com/Perl/perl5/commit/e9a908c9e8acd172c6c36d6cd556432b4821c691 (this is "29796") <dipsy> [14:53:32] [ unfreed threads should trigger cleanup veto · Perl/perl5@e9a908c · GitHub ] <Exodist> [14:54:12] wow. it is intentional behavior <hobbs> [14:54:21] "The thread pool struct is allocated in the main interpreter, so don't clean that up if any threads remain, regardless of what state they are in" <hobbs> [14:56:11] between 5.9.4 and 5.9.5, so haarg hit the mark <haarg> [14:59:17] well, "intentional" <hobbs> [15:00:10] yeah, more of a cop-out than a fix <hobbs> [15:00:14] I mean, segfaults are bad and all * Exodist [15:01:31] wonders how many things out there will actually detach a thread and let it hang while the main thread exits. <hobbs> [15:02:13] it seems less than useful, since the detached thread will just unexpectedly terminate anyway <Exodist> [15:02:19] yeah.

Wed May 18 13:12:38 2016 jdhedden [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #114468] 07_lock.t leaks temp files, and other bad things
Date:	Wed, 18 May 2016 13:11:52 -0400
To:	bug-Thread-Queue [...] rt.cpan.org
From:	"Jerry D. Hedden" <jdhedden [...] cpan.org>

Wow. This is something that needs to be documented in the 'threads' POD. Thanks for the info. I'll handle both in the next few days. On Wed, May 18, 2016 at 1:01 PM, Chad Granum via RT < bug-Thread-Queue@rt.cpan.org> wrote: Show quoted text

> Queue: Thread-Queue > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=114468 > > > Here is a snippet of the detective work: > > <haarg> [14:45:07] 5.8 sometimes it works, sometimes it segfaults, > sometimes it complains about 2 threads running at exit > <haarg> [14:45:40] and the other various things that threads always do > <haarg> [14:46:42] monkeypatching threads is really ugly, but as far as i > can tell there isn't any other way to get the information needed > <haarg> [14:47:07] so it would need some other workaround > <hobbs> [14:52:25] > github.com/Perl/perl5/commit/ae3fba3defe4ed89f6b20b720860824178f77f52 > <hobbs> [14:52:30] > https://github.com/Perl/perl5/commit/ae3fba3defe4ed89f6b20b720860824178f77f52 > <dipsy> [14:52:31] [ further refinement to #29796 (cleanup veto) · > Perl/perl5@ae3fba3 · GitHub ] > <hobbs> [14:53:31] > https://github.com/Perl/perl5/commit/e9a908c9e8acd172c6c36d6cd556432b4821c691 > (this is "29796") > <dipsy> [14:53:32] [ unfreed threads should trigger cleanup veto · > Perl/perl5@e9a908c · GitHub ] > <Exodist> [14:54:12] wow. it is intentional behavior > <hobbs> [14:54:21] "The thread pool struct is allocated in the main > interpreter, so don't clean that up if any threads remain, regardless of > what state they are in" > <hobbs> [14:56:11] between 5.9.4 and 5.9.5, so haarg hit the mark > <haarg> [14:59:17] well, "intentional" > <hobbs> [15:00:10] yeah, more of a cop-out than a fix > <hobbs> [15:00:14] I mean, segfaults are bad and all > * Exodist [15:01:31] wonders how many things out there will actually > detach a thread and let it hang while the main thread exits. > <hobbs> [15:02:13] it seems less than useful, since the detached thread > will just unexpectedly terminate anyway > <Exodist> [15:02:19] yeah. >

Wed May 18 13:15:56 2016 haarg [...] haarg.org - Correspondence added

On Wed May 18 12:46:43 2016, JDHEDDEN wrote: Show quoted text

> On 2016-05-18 12:02:06, EXODIST wrote:

> > This test detaches a thread and lets the parent thread end before the > > detached thread.

> > I don't think that is the case. The detached thread locks the queue > that is being tested (line 34). This lock is not released until the > thread exits its subroutine at which point is will start to be > destroyed. > > Meanwhile, the main thread is blocked from reading the queue until the > lock is released (line 46). Following that it does one more yield > (line 48) to allow for the thread to be destroyed before itself > exiting. > > Do you have any evidence that indicates the main thread exits before > the detached thread is destroyed? >

> > This [main thread exiting with detached threads] causes perl to skip > > global > > destruction.

> > I am not aware that this is the case. Can you provide > documentation/evidence regarding this?

Attached is a script that demonstrates the issue with global destruction and detached threads. Depending on the timing, you can get 2 or 0 messages.

Subject:

thread-detach-gd.pl

use strict; use warnings; use threads; sub ShowDestroy::DESTROY { warn "destroyed in thread " . threads->tid; } our $guff = bless {}, 'ShowDestroy'; threads->create(sub{ 1 })->detach;

Sun May 22 07:29:41 2016 jdhedden [...] cpan.org - Correspondence added

On 2016-05-18 12:02:06, EXODIST wrote: Show quoted text

> https://metacpan.org/source/JDHEDDEN/Thread-Queue-3.09/t/07_lock.t > > This test detaches a thread and lets the parent thread end before the > detached thread. This causes perl to skip global destruction. Without > global destruction you get some nasty things like temp file and SHM > leaks. > > Changing the test so that it does not detach, but instead eventually > joins, will allow global destruction to take place and things don't > leak anymore.

Fixed in v3.11. Thanks.

Sun May 22 07:29:50 2016 jdhedden [...] cpan.org - Status changed from 'open' to 'resolved'

Sun May 22 07:29:51 2016 jdhedden [...] cpan.org - Fixed in 3.11 added