Re: Bug#839905: Bug#840293: pkg-kde-tools: pkgkde-symbolshelper broken by perl 5.24

[Guillem Jover <guillem@debian.org>]

Control: clone -1 -2
Control: reassign -2 libdpkg-perl
Control: retitle -2 libdpkg-perl: Dpkg::IPC::spawn immediately closes FH after dup

Hi!

[ Was pointed to this bug by Stuart Prescott. ]

On Thu, 2016-10-13 at 23:30:09 +0200, gregor herrmann wrote:
> On Thu, 13 Oct 2016 17:13:09 -0300, Lisandro Damián Nicanor Pérez Meyer wrote:
> > Steps to reproduce:
> > 
> > apt-get source -t experimental qtdeclarative-opensource-src
> > cd qtdeclarative-opensource-src-5.7.0/
> > getbuildlog qtdeclarative-opensource-src last
> > pkgkde-symbolshelper batchpatch -v 5.7.0 *.log
> 
> Thanks!
> That makes it indeed easier to reproduce :)

Indeed!

> So, my findings:
> - the error "reopen stdout: Bad file descriptor" happens in
>   /usr/share/perl5/Debian/PkgKde/SymbolsHelper/Patching.pm line 246:
>   wait_child($pid, nocheck => 1);
> - that's when the formerly assembled $pid gets executed
> - looking at $pid above, I notice that both to_handle and
>   error_to_handle are set to $outfile (a temp file created before)
> - just out of curiosity, I commented out the error_to_handle line
>   (241), and now `pkgkde-symbolshelper batchpatch -v 5.7.0 *.log'
>   passes
> - running `pkgkde-symbolshelper batchpatch -v 5.7.0 *.log' again
>   fails with failed hunk errors from patch, which also doesn't look
>   so bad
> 
> Not sure where this leaves us, especially since I neither know
> Dpkg::IPC nor understand the apply() sub in
> Debian::PkgKde::SymbolsHelper::Patching.

> (And I also don't know where exactly the problem / changed behaviour
> with perl 5.24 lies. Since the spawn/wait_pid functions are from
> Dpkg::IPC, I guess looking into Dpkg::IPC might be interesting.)

The problem is that spawn() is closing the passed filehandle just
after duping it to STDOUT and then STDERR instead of queueing it for
deferred closure. I'm also not sure why this didn't trigger before?
Perhaps perl was delaying the filehandle closure?

I'll include the attached patch fpr libdpkg-perl in the next dpkg
release, but in the interim you might want to workaround this in
pkg-kde-tools, by duping the $output filehandle before passing it
to spawn().

Thanks,
Guillem

diff --git i/scripts/Dpkg/IPC.pm w/scripts/Dpkg/IPC.pm
index 5172540..3dfbde9 100644
--- i/scripts/Dpkg/IPC.pm
+++ w/scripts/Dpkg/IPC.pm
@@ -282,7 +282,8 @@ sub spawn {
 	} elsif ($opts{from_handle}) {
 	    open(STDIN, '<&', $opts{from_handle})
 		or syserr(g_('reopen stdin'));
-	    close($opts{from_handle}); # has been duped, can be closed
+	    # has been duped, can be closed
+	    push @{$opts{close_in_child}}, $opts{from_handle};
 	}
 	# Redirect STDOUT if needed
 	if ($opts{to_file}) {
@@ -291,7 +292,8 @@ sub spawn {
 	} elsif ($opts{to_handle}) {
 	    open(STDOUT, '>&', $opts{to_handle})
 		or syserr(g_('reopen stdout'));
-	    close($opts{to_handle}); # has been duped, can be closed
+	    # has been duped, can be closed
+	    push @{$opts{close_in_child}}, $opts{to_handle};
 	}
 	# Redirect STDERR if needed
 	if ($opts{error_to_file}) {
@@ -300,7 +302,8 @@ sub spawn {
 	} elsif ($opts{error_to_handle}) {
 	    open(STDERR, '>&', $opts{error_to_handle})
 	        or syserr(g_('reopen stdout'));
-	    close($opts{error_to_handle}); # has been duped, can be closed
+	    # has been duped, can be closed
+	    push @{$opts{close_in_child}}, $opts{error_to_handle};
 	}
 	# Close some inherited filehandles
 	close($_) foreach (@{$opts{close_in_child}});