Bug #57990 for IPC-Run: Bugfix for memory leak

The internals seem a little dodgy if this is a safe or appropriate thing to do. There's not spectacular documentation on the _cleanup subroutines. What I do know is that the patch breaks the test suite ATM.... $>make test PERL_DL_NONLAZY=1 /usr/local/cpanel/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/97_meta.t .................. ok t/98_pod.t ................... ok t/98_pod_coverage.t .......... ok t/99_perl_minimum_version.t .. skipped: no minimum perl version could be determined t/adopt.t .................... skipped: adopt not implemented yet t/binmode.t .................. ok t/bogus.t .................... ok t/eintr.t .................... ok t/filter.t ................... ok t/harness.t .................. ok t/io.t ....................... ok t/kill_kill.t ................ ok t/parallel.t ................. ok t/pty.t ...................... # IO::Tty 1.10, IO::Pty 1.10 t/pty.t ...................... ok t/pump.t ..................... ok t/run.t ...................... 134/268 ack Can't use an undefined value as a SCALAR reference at /Users/toddr/projects/IPC-Run/blib/lib/IPC/Run.pm line 2449. # Looks like you planned 268 tests but ran 241. # Looks like your test exited with 9 just after 241. t/run.t ...................... Dubious, test returned 9 (wstat 2304, 0x900) Failed 27/268 subtests t/signal.t ................... ok t/timeout.t .................. 9/26 # Failed test at t/timeout.t line 67. # got: 'ack Can't use an undefined value as a SCALAR reference at /Users/toddr/projects/IPC-Run/blib/lib/IPC/Run.pm line 2449. # ' # expected: '(?-xism:IPC::Run: timeout)' # Failed test at t/timeout.t line 69. # Failed test at t/timeout.t line 70. # Failed test at t/timeout.t line 75. # got: '0' # expected: '>= 1' t/timeout.t .................. 26/26 # Looks like you failed 4 tests of 26. t/timeout.t .................. Dubious, test returned 4 (wstat 1024, 0x400) Failed 4/26 subtests t/timer.t .................... ok t/utf8.t ..................... ok t/win32_compile.t ............ ok t/windows_search_path.t ...... ok Test Summary Report ------------------- t/run.t (Wstat: 2304 Tests: 241 Failed: 0) Non-zero exit status: 9 Parse errors: Bad plan. You planned 268 tests but ran 241. t/timeout.t (Wstat: 1024 Tests: 26 Failed: 4) Failed tests: 19, 21-23 Non-zero exit status: 4 Files=22, Tests=684, 13 wallclock secs ( 0.16 usr 0.07 sys + 1.81 cusr 0.58 csys = 2.62 CPU) Result: FAIL Failed 2/22 test programs. 4/684 subtests failed. make: *** [test_dynamic] Error 255

FWIW, this is a solution I'm trying locally and tests do pass. I tried the other patch but it seems to prevent rerunning a harness. I can't say for certain if this change is safe, I am not familiar with all of the IPC::Run internals. --- a/lib/IPC/Run.pm +++ b/lib/IPC/Run.pm @@ -1136,4 +1136,10 @@ sub DESTROY { POSIX::close $self->{DEBUG_FD} if defined $self->{DEBUG_FD}; $self->{DEBUG_FD} = undef; + + for my $kid ( @{$self->{KIDS}} ) { + for my $op ( @{$kid->{OPS}} ) { + @{$op->{FILTERS}} = (); + } + } } @@ -2656,5 +2662,5 @@ sub _do_kid_and_exit { _write $self->{SYNC_WRITER_FD}, $@; ## Avoid DESTROY. - POSIX::exit 1; + POSIX::_exit 1; } @@ -2673,10 +2679,10 @@ sub _do_kid_and_exit { $kid->{VAL} = undef; - ## Use POSIX::exit to avoid global destruction, since this might + ## Use POSIX::_exit to avoid global destruction, since this might ## cause DESTROY() to be called on objects created in the parent ## and thus cause double cleanup. For instance, if DESTROY() unlinks ## a file in the child, we don't want the parent to suddenly miss ## it. - POSIX::exit 0; + POSIX::_exit 0; } @@ -4345,5 +4351,5 @@ The second problem is that Perl's DESTROY subs and other on-exit cleanup gets run in the child process. If objects are instantiated in the parent before the child is forked, the the DESTROY will get run once in the parent and once in -the child. When coprocess subs exit, POSIX::exit is called to work around this, +the child. When coprocess subs exit, POSIX::_exit is called to work around this, but it means that objects that are still referred to at that time are not cleaned up. So setting package vars or closure vars to point to objects that

Ticket migrated to github as https://github.com/toddr/IPC-Run/issues/86