Skip Menu |

This queue is for tickets about the PDF-Create CPAN distribution.

Report information
The Basics
Id: 119292
Status: resolved
Priority: 0/
Queue: PDF-Create

People
Owner: MANWAR [...] cpan.org
Requestors: SREZIC [...] cpan.org
Cc: andy [...] andybev.com
AdminCc:

Bug Information
Severity: Important
Broken in:
  • 1.36
  • 1.37
  • 1.38
  • 1.39
Fixed in: 1.41



CC: andy [...] andybev.com
Subject: "Bad file descriptor" error on closing fh
One of my applications started to fail with "Bad file descriptor" when calling close() on a filehandle which before was operated on by PDF::Create. The application does roughly the following: open my $OUT, ">", $file or die $!; # ... now it chooses which of the available backends (GD, PDF::Create, Cairo...) should be used ... # if it is PDF::Create, then roughly call it like this my $pdf = PDF::Create->new(fh => $OUT, ...) # ... do work on $pdf ... $pdf->close # ... now we're out of the backend module and again in the main function where $OUT was created ... close $OUT or die $!; With the change for https://rt.cpan.org/Ticket/Display.html?id=118764 the last code line started to fail. It seems there's a discrepancy what to expect from PDF::Create's close() method. In my case, I need a method call which does all the flush work, but which does *not* close the filehandle — as the filehandle is opened outside the module, I want also to close it outside the module. On the other hand, the name "close" might suggest that the used filehandle is also closed. To solve this problem, I propose to split the functionality of the current close() method: a new flush() method would do all the add_* method calls, but won't close the filehandle. The close() method would now just call flush() and then close the filehandle.
Subject: Re: [rt.cpan.org #119292] "Bad file descriptor" error on closing fh
Date: Sun, 18 Dec 2016 20:34:36 +0000
To: bug-PDF-Create [...] rt.cpan.org
From: Andrew Beverley <andy [...] andybev.com>
Show quoted text
> With the change for https://rt.cpan.org/Ticket/Display.html?id=118764 > the last code line started to fail.
Ah, sorry about that. Show quoted text
> It seems there's a discrepancy what to expect from PDF::Create's close > () method.
Yes, I think I did look at the docs, and it didn't state anything different for close() on a filename parameter vs close() on a filehandle. Show quoted text
> as the filehandle is opened outside the module, I want also to close > it outside the module.
I can now see why you would want that. I have to admit it didn't occur to me - I just ran into trouble when switching from filenames to filehandles and got the unexpected behaviour. Show quoted text
> To solve this problem, I propose to split the functionality of the > current close() method: a new flush() method would do all the add_* > method calls, but won't close the filehandle. The close() method > would now just call flush() and then close the filehandle.
+1
Hi @SREZIC, Thanks for raising the issue and a valid one. I agree with your proposed solution. I will try to get it patched by tomorrow. Best Regards, Mohammad S Anwar
Hi @SREZIC, Having checked the code closely, I think I should put the line back what it was before as below: $self->{'fh'}->close if defined $self->{'fh'} && defined $self->{'filename'}; This is safe as close() will only close the file handle if it was internally created using the filename provided. In all other cases, the file handle will remain open and would expect the user to explicitly close it when done. What do you think? However this would break Andy's code, for that I suggest method close_all() that would do all of close() and explicitly close the filehandle provided when done. Above all document this behaviour, so there is no confusion in the air, Best Regards, Mohammad S Anwar
CC: SREZIC [...] cpan.org
Subject: Re: [rt.cpan.org #119292] "Bad file descriptor" error on closing fh
Date: Mon, 19 Dec 2016 16:27:51 +0000
To: bug-PDF-Create [...] rt.cpan.org
From: Andrew Beverley <andy [...] andybev.com>
Show quoted text
> This is safe as close() will only close the file handle if it was > internally created using the filename provided. In all other cases, > the file handle will remain open and would expect the user to > explicitly close it when done. > > What do you think?
I personally prefer SREZIC's proposal, otherwise you have a method called "close" which doesn't actually do any closing (or rather sometimes closes and sometimes doesn't). Show quoted text
> However this would break Andy's code,
That's not really a problem - I can adjust my code accordingly. Show quoted text
> Above all document this behaviour, so there is no confusion in the air,
That's the key thing. As long as it's all documented, I'll adjust accordingly. Andy
RT-Send-CC: andy [...] andybev.com
Hi Andy, I am happy either way :-) The only thing I am worried about is, if I am going to upset any user, like @SREZIC, if we change the original behaviour prior to your patch i.e. close() will only close internal file handle and leave any external file handle for the user to deal with it. As per @SREZIC, close() will now close any internal/supplied file handle at the end. Splitting the functionality of close() is really nice and safe. That way, you can call the new method flush() and deal with the file handle externally. Last but not the least, document method close() with regard to external file handle. Best Regards, Mohammad S Anwar
Subject: Re: [rt.cpan.org #119292] "Bad file descriptor" error on closing fh
Date: Mon, 19 Dec 2016 20:03:10 +0000
To: bug-PDF-Create [...] rt.cpan.org
From: Andrew Beverley <andy [...] andybev.com>
Fair one, I guess you're stuck with the API, even if it's not ideal. Maybe just document it better - I can easily add a $fh->close to my app. Thanks, Andy
Resolved.