Bug #49915 for POE: t/90_regression/kjeldahl-stop-start fails on Win32

Tue Sep 22 13:54:08 2009 acferen [...] yahoo.com - Ticket created

Subject:

t/90_regression/kjeldahl-stop-start fails on Win32

svn version : 2697 S:\poe\trunk\poe>perl -Ilib t/90_regression/kjeldahl-stop-start.t # This test can take up to 2 seconds 1..21 # Without USE_SIGCHLD not ok 1 - Expecting to close # Failed test 'Expecting to close' # at t/90_regression/kjeldahl-stop-start.t line 137. # got: '0' # expected: '1' ok 2 - Child 1 exit detected. not ok 3 - Expecting to close # Failed test 'Expecting to close' # at t/90_regression/kjeldahl-stop-start.t line 137. # got: '0' # expected: '1' ok 4 - Child 2 exit detected. not ok 5 - Expecting to close # Failed test 'Expecting to close' # at t/90_regression/kjeldahl-stop-start.t line 137. # got: '0' # expected: '1' ok 6 - Child 3 exit detected. ok 7 - all children have exited # With USE_SIGCHLD not ok 8 - Expecting to close # Failed test 'Expecting to close' # at t/90_regression/kjeldahl-stop-start.t line 137. # got: '0' # expected: '1' ok 9 - Child 4 exit detected. not ok 10 - Expecting to close # Failed test 'Expecting to close' # at t/90_regression/kjeldahl-stop-start.t line 137. # got: '0' # expected: '1' ok 11 - Child 5 exit detected. not ok 12 - Expecting to close # Failed test 'Expecting to close' # at t/90_regression/kjeldahl-stop-start.t line 137. # got: '0' # expected: '1' ok 13 - Child 6 exit detected. ok 14 - all children have exited ok 15 - Sane exit # Looks like you planned 21 tests but ran 15. # Looks like you failed 6 tests of 15 run.

Tue Sep 22 14:16:39 2009 acferen [...] yahoo.com - Correspondence added

Here is a patch that works. I don't (yet) know why it works. The change to the "Program => ..." option works with (or without) sleep. I put the sleep in as the moral equivalent of spawn_child printing "DONE" after the delay timer expires. The change to chld_stdout is reqired to ignore whitespace (line endings) that may slip past POE::Filter::Line I left spawn_child in the test, but with this patch it could be removed.

Index: t/90_regression/kjeldahl-stop-start.t =================================================================== --- t/90_regression/kjeldahl-stop-start.t (revision 2697) +++ t/90_regression/kjeldahl-stop-start.t (working copy) @@ -56,8 +56,7 @@ foreach my $n (1 .. $N) { DEBUG and diag "$$: Launch child $n"; my $w = POE::Wheel::Run->new( - Program => \&spawn_child, - ProgramArgs => [ $sleep ], + Program => qq{ perl -e "sleep($sleep); print qq{DONE\\n}" }, StdoutEvent => 'chld_stdout', StderrEvent => 'chld_stdin', CloseEvent => 'chld_close' @@ -73,6 +72,7 @@ my ($heap, $line, $wid) = @_[HEAP, ARG0, ARG1]; my $W = $heap->{W}{$wid}; die "Unknown wheel $wid" unless $W; + $line =~ s/(DONE)\s*/$1/; is( $line, 'DONE', "stdout from $wid" ); if( $line eq 'DONE' ) { my $data = $heap->{PID2W}{ $W->PID };

Tue Sep 22 14:16:41 2009 acferen [...] yahoo.com - Status changed from 'new' to 'open'

Thu Oct 01 01:20:27 2009 RCAPUTO [...] cpan.org - Correspondence added

I'm seeing the failure here, so thanks for pointing it out. The patch probably works because the mechanisms for fork/exec and fork/call are significantly different. Whatever's going awry in fork/call is peachy-keen in the fork/exec code path. So... the patch helps narrow down where the problem lies, but it would only mask the real problem. I can't use it for production releases, but it'll be useful for debugging. On Tue Sep 22 14:16:39 2009, acferen@yahoo.com wrote: Show quoted text

> Here is a patch that works. I don't (yet) know why it works. > > The change to the "Program => ..." option works with (or without) > sleep. I put the sleep in as the moral equivalent of spawn_child > printing "DONE" after the delay timer expires. > > The change to chld_stdout is reqired to ignore whitespace (line > endings) that may slip past POE::Filter::Line > > I left spawn_child in the test, but with this patch it could be removed.

Thu Oct 01 12:31:49 2009 acferen [...] yahoo.com - Correspondence added

On Thu Oct 01 01:20:27 2009, RCAPUTO wrote: Show quoted text

> I'm seeing the failure here, so thanks for pointing it out. > > The patch probably works because the mechanisms for fork/exec and > fork/call are significantly different. Whatever's going awry in > fork/call is peachy-keen in the fork/exec code path. > > So... the patch helps narrow down where the problem lies, but it would > only mask the real problem. I can't use it for production releases, but > it'll be useful for debugging.

That logic makes total sense and is the main reason I submitted the patch. Would you consider applying the part of the patch to deal with whitespace after 'DONE'. As I understand it that will be needed for Win32 regardless of the rest of the resolution. I've attached a new patch with just that one line.

Index: t/90_regression/kjeldahl-stop-start.t =================================================================== --- t/90_regression/kjeldahl-stop-start.t (revision 2706) +++ t/90_regression/kjeldahl-stop-start.t (working copy) @@ -73,6 +73,7 @@ my ($heap, $line, $wid) = @_[HEAP, ARG0, ARG1]; my $W = $heap->{W}{$wid}; die "Unknown wheel $wid" unless $W; + $line =~ s/(DONE)\s*/$1/; is( $line, 'DONE', "stdout from $wid" ); if( $line eq 'DONE' ) { my $data = $heap->{PID2W}{ $W->PID };

Thu Oct 01 23:52:11 2009 RCAPUTO [...] cpan.org - Correspondence added

On Thu Oct 01 12:31:49 2009, acferen@yahoo.com wrote: Show quoted text

> > Would you consider applying the part of the patch to deal with > whitespace after 'DONE'. As I understand it that will be needed for > Win32 regardless of the rest of the resolution. > > I've attached a new patch with just that one line.

Applied as revision 2708. Thanks. :)

Tue Oct 06 00:04:34 2009 RCAPUTO [...] cpan.org - Correspondence added

Strangely, this test isn't part of the MANIFEST. I vaguely recall that we weren't distributing it because it failed on Windows. Anyway, since it's a known issue, I've added code to skip the tests on MSWin32 and added it to the MANIFEST. You're welcome to debug it and send patches, but I'm afraid I don't know fork() well enough on MSWin32 to get this to work.

Tue Oct 06 00:04:35 2009 RCAPUTO [...] cpan.org - Status changed from 'open' to 'resolved'