[perl #122112] refcnt: fd -1 < 0 with MIME::Lite

On Tue Jul 08 03:02:17 2014, vsespb wrote:
> On Mon Jul 07 16:35:06 2014, tonyc wrote:
> > On Mon Jul 07 12:48:51 2014, vsespb wrote:
> > > So instead
> > >
> > > if (fd < 0) {
> > >       /* the handle has already been closed */
> > >       return -1;
> > > }
> > >
> > > Perl_my_pclose should continue and run wait4pid loop, but
> > > PerlIO_close
> > > should be skipped.
> >
> > Without the original fd we can't look up the pid to wait on.
> >
> 
> We need make function re-entrant. Can save PID for later user in
> global varibale.
> 
> > We can't re-order the close/wait to wait/close, since if we're
> > writing
> > to the child, it may be waiting for EOF (eg. a filter) before
> > exiting.
> >
> yes, right.
> 
> 
> Here is prototype. All tests pass and original problem fixed.
> 
...
> 
> but real patch should also keep relation between PID and FH and work
> even if my_close(FILE1) called but then my_close(FILE2) called from
> inside signal handler.

Another problem I can see with the current my_pclose() code - the second time we enter my_pclose() for our sample code, the PerlIO object has already been freed by PerlIO_close(), so we're accessing a freed object.  Because of the way PerlIO allocates memory, this isn't picked up by valgrind.

I can think of a few different possibilities to fix the problem:

a) implement a half-close operation for PerlIO that closes the handle but doesn't release the PerlIO object.  my_pclose() would half-close the object, perform the wait, then do the full close which releases the PerlIO object.  I'm not sure how much work this is.

b) keep a linked list (or other structure, but hopefully we don't have a lot of them) of closing-in-progress pipe PerlIO handles, my_pclose() checks if the handle is on the list. If it is, it bypasses the PerlIO_close(), if not, call PerlIO_close() and add an entry to the list with the PerlIO object pointer[1] and pid.  After the wait() loop is complete remove the entry from the list.  I think this might be the correct solution.  Locking may need to be an issue as the linked list needs to be visible cross-thread as some handles are visible cross-thread.  This is the multi-handle version of your solution.

c) implement something close to the vms/vms.c implementation of my_popen()/my_pclose() which keeps a fairly complex data structure for each pipe.  Overkill here I think.

Tony

[1] this is undefined behaviour, but less so than following the pointer, I expect

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=122112

• [perl #122112] refcnt: fd -1 < 0 with MIME::Lite by Tony Cook via RT