Skip Menu |

This queue is for tickets about the POE CPAN distribution.

Report information
The Basics
Id: 50068
Status: resolved
Priority: 0/
Queue: POE

People
Owner: Nobody in particular
Requestors: acferen [...] yahoo.com
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.269
Fixed in: (no value)



Subject: Memory leak in POE-Wheel-Run (Win32)
There is a small memory leak with every new POE::Wheel::Run. To reproduce run the attached script and observe the memory usage in the windows task manager.
Subject: wr_leak.pl
#!/usr/bin/perl use warnings; use strict; sub POE::Kernel::ASSERT_DEFAULT () { 1 } use POE; use POE::Wheel::Run; use POE::Filter::Line; POE::Session->create( inline_states => { _start => \&on_start, _stop => sub { print "main done"; }, got_child_stdout => \&on_child_stdout, got_child_stderr => \&on_child_stderr, got_child_close => \&on_child_close, got_child_signal => \&on_child_signal, pulse => \&pulse, event_sigint => sub { warn "Shutting down gracefully...\n"; $_[KERNEL]->sig_handled(); $_[KERNEL]->alarm_remove_all(); $_[HEAP]->{done} = 1; return 0; }, } ); POE::Kernel->run(); exit 0; sub on_start { $_[KERNEL]->sig( INT => 'event_sigint' ); $_[KERNEL]->delay( pulse => 1); } # Wheel event, including the wheel's ID. sub on_child_stdout { my ($stdout_line, $wheel_id) = @_[ARG0, ARG1]; my $child = $_[HEAP]{children_by_wid}{$wheel_id}; print "pid ", $child->PID, " STDOUT: $stdout_line\n"; } # Wheel event, including the wheel's ID. sub on_child_stderr { my ($stderr_line, $wheel_id) = @_[ARG0, ARG1]; my $child = $_[HEAP]{children_by_wid}{$wheel_id}; print "pid ", $child->PID, " STDERR: $stderr_line\n"; } # Wheel event, including the wheel's ID. sub on_child_close { my $wheel_id = $_[ARG0]; my $child = delete $_[HEAP]{children_by_wid}{$wheel_id}; # May have been reaped by on_child_signal(). unless (defined $child) { print "wid $wheel_id closed all pipes.\n"; return; } print "pid ", $child->PID, " closed all pipes.\n"; delete $_[HEAP]{children_by_pid}{$child->PID}; $_[KERNEL]->yield( 'pulse' ) unless $_[HEAP]->{done}; } sub on_child_signal { print "pid $_[ARG1] exited with status $_[ARG2].\n"; my $child = delete $_[HEAP]{children_by_pid}{$_[ARG1]}; #$_[KERNEL]->sig_child($_[ARG1]); # May have been reaped by on_child_close(). return unless defined $child; delete $_[HEAP]{children_by_wid}{$child->ID}; $_[KERNEL]->yield( 'pulse' ) unless $_[HEAP]->{done}; } sub pulse { my $child; eval { $child = POE::Wheel::Run->new( Program => 'perl -e "sleep 2"', StdoutEvent => "got_child_stdout", StderrEvent => "got_child_stderr", CloseEvent => "got_child_close", ); }; if ($@) { warn $@; } $_[KERNEL]->sig_child($child->PID, "got_child_signal"); # Wheel events include the wheel's ID. $_[HEAP]{children_by_wid}{$child->ID} = $child; # Signal events include the process ID. $_[HEAP]{children_by_pid}{$child->PID} = $child; print( "Child pid ", $child->PID, " started as wheel ", $child->ID, ".\n" ); }
From: acferen [...] yahoo.com
Attached is a patch that fixes the leak. I'm not thrilled with this patch. It does fix the leak I am seeing and all tests continue to pass, but... The calls to Win32::Console::_SetStdHandle seemed to be critical at some point. Perhaps they are still needed in pre 5.8 perl? As near as I can tell all they are doing for me is leaking memory. Maybe they are needed in some corner case not not in the POE test suite or my application.
Index: trunk/poe/lib/POE/Wheel/Run.pm =================================================================== --- trunk/poe/lib/POE/Wheel/Run.pm (revision 2700) +++ trunk/poe/lib/POE/Wheel/Run.pm (working copy) @@ -390,7 +390,7 @@ print $sem_pipe_write "go\n"; close $sem_pipe_write; - if (POE::Kernel::RUNNING_IN_HELL) { + if (0 && POE::Kernel::RUNNING_IN_HELL) { # The Win32 pseudo fork sets up the std handles in the child # based on the true win32 handles For the exec these get # remembered, so manipulation of STDIN/OUT/ERR is not enough.
On Mon Sep 28 10:55:26 2009, acferen@yahoo.com wrote: Show quoted text
> There is a small memory leak with every new POE::Wheel::Run. To > reproduce run the attached script and observe the memory usage in the > windows task manager.
Well, I can confirm the leak with your test program, but your patch was broken when I applied some of your other patches. Can you re-diff against the latest repository version? Currently r2706.
On Thu Oct 01 02:18:48 2009, RCAPUTO wrote: Show quoted text
> On Mon Sep 28 10:55:26 2009, acferen@yahoo.com wrote:
> > There is a small memory leak with every new POE::Wheel::Run. To > > reproduce run the attached script and observe the memory usage in the > > windows task manager.
> > Well, I can confirm the leak with your test program, but your patch was > broken when I applied some of your other patches. Can you re-diff > against the latest repository version? Currently r2706.
patch against r2706 attached.
Index: trunk/poe/lib/POE/Wheel/Run.pm =================================================================== --- trunk/poe/lib/POE/Wheel/Run.pm (revision 2706) +++ trunk/poe/lib/POE/Wheel/Run.pm (working copy) @@ -419,7 +419,7 @@ close $sem_pipe_write; } - if (POE::Kernel::RUNNING_IN_HELL) { + if (0 && POE::Kernel::RUNNING_IN_HELL) { # The Win32 pseudo fork sets up the std handles in the child # based on the true win32 handles For the exec these get # remembered, so manipulation of STDIN/OUT/ERR is not enough.
On Thu Oct 01 08:47:49 2009, acferen@yahoo.com wrote: Show quoted text
> On Thu Oct 01 02:18:48 2009, RCAPUTO wrote:
> > On Mon Sep 28 10:55:26 2009, acferen@yahoo.com wrote:
> > > There is a small memory leak with every new POE::Wheel::Run. To > > > reproduce run the attached script and observe the memory usage in the > > > windows task manager.
Show quoted text
> patch against r2706 attached.
I've asked the mailing list to help me evaluate the patch and its side effects. The code your patch bypasses seems important, and I don't know enough about Windows APIs to tell whether it's actually needed. It might be needed on certain versions of Perl and/or Windows, for example. Hopefully we can get more Windows people together to make sure the right thing's done.
By itself, calls to Win32::Console::_setStdHandle() don't seem to be the problem. This test program doesn't leak memory: #!perl use warnings; use strict; use Win32::Console; use Win32API::File qw(FdGetOsFHandle); while (1) { Win32::Console::_SetStdHandle( STD_INPUT_HANDLE(), FdGetOsFHandle(fileno(STDIN)) ); }
This test program quickly leaks memory. It's not fork() by itself, and it's not Win32::Console::_SetStdHandle(), but it is both of them together. #!perl use warnings; use strict; use Win32::Console; use Win32API::File qw(FdGetOsFHandle); TRY: while (1) { my $pid = fork(); # fork() failed. next TRY unless defined $pid; # Parent. if ($pid) { # Wait for child. waitpid($pid, 0); next TRY; } # Child. Win32::Console::_SetStdHandle( STD_INPUT_HANDLE(), FdGetOsFHandle(fileno(STDIN)) ); exit(0); }
Win32::Console is leaking, not POE. I've opened a ticket against Win32::Console.
I meant to resolve this ticket, not reply. My work here is done, unless someone has an alternative to Win32::Console that does the same thing without leaking.
From: acferen [...] yahoo.com
On Thu Dec 31 18:44:57 2009, RCAPUTO wrote: Show quoted text
> I meant to resolve this ticket, not reply. My work here is done, unless > someone has an alternative to Win32::Console that does the same thing > without leaking.
Here is a patch based on the work around suggested in https://rt.cpan.org/Public/Bug/Display.html?id=53264#txn-714849. Ran this overnight with no trouble. -Andrew
Subject: wheel_run.patch
Index: trunk/poe/lib/POE/Wheel/Run.pm =================================================================== --- trunk/poe/lib/POE/Wheel/Run.pm (revision 2796) +++ trunk/poe/lib/POE/Wheel/Run.pm (working copy) @@ -15,6 +15,9 @@ use POE qw( Wheel Pipe::TwoWay Pipe::OneWay Driver::SysRW Filter::Line ); use base qw(POE::Wheel); +my $STD_INPUT_HANDLE; +my $STD_OUTPUT_HANDLE; +my $STD_ERROR_HANDLE; BEGIN { die "$^O does not support fork()\n" if $^O eq 'MacOS'; @@ -44,6 +47,10 @@ eval { require Win32; Win32->import() }; if ($@) { die "Win32.pm needed for POE::Wheel::Run on $^O:\n$@" } + + $__PACKAGE__::STD_INPUT_HANDLE = STD_INPUT_HANDLE(); + $__PACKAGE__::STD_OUTPUT_HANDLE = STD_OUTPUT_HANDLE(); + $__PACKAGE__::STD_ERROR_HANDLE = STD_ERROR_HANDLE(); } # Determine the most file descriptors we can use. @@ -1123,17 +1130,17 @@ # alternatives? Win32::Console::_SetStdHandle( - STD_INPUT_HANDLE(), + $__PACKAGE__::STD_INPUT_HANDLE, FdGetOsFHandle(fileno($stdin_read)) ); Win32::Console::_SetStdHandle( - STD_OUTPUT_HANDLE(), + $__PACKAGE__::STD_OUTPUT_HANDLE, FdGetOsFHandle(fileno($stdout_write)) ); Win32::Console::_SetStdHandle( - STD_ERROR_HANDLE(), + $__PACKAGE__::STD_ERROR_HANDLE, FdGetOsFHandle(fileno($stderr_write)) ); }
On Wed Feb 10 11:43:35 2010, acferen@yahoo.com wrote: Show quoted text
> On Thu Dec 31 18:44:57 2009, RCAPUTO wrote:
> > I meant to resolve this ticket, not reply. My work here is done, unless > > someone has an alternative to Win32::Console that does the same thing > > without leaking.
> > Here is a patch based on the work around suggested in > https://rt.cpan.org/Public/Bug/Display.html?id=53264#txn-714849. > > Ran this overnight with no trouble.
Thank you for the patch. I've applied a modified version of it as revision 2801, and it'll appear in the next POE release, sometime this weekend. My modifications remove the __PACKAGE__:: from the variable names. Variables with package qualifiers are globals, and the variables were initally declared as lexicals with my().
Resolved in revision 2801. Thank you, everyone.