Re: unbuffered fread() deadlock

[Jeff Johnston <jjohnstn at redhat dot com>]

Andre,

 It's actually more complicated.

 The problem is that the read operation is allowing an fp lock before 
an sfp lock.  The sfp lock is meant to occur before any fp locks and in 
all other cases, this is true (e.g. fclose, fopen, fflush(null), 
fcloseall).  In the read case, we are allowing the fp lock first which 
is recipe for a deadlock.  Even if we kludge this particular situation 
you can see that if a read operation starts while doing an fcloseall(), 
we are back in the same boat.

 Thus said, the answer is to sfp_lock any read operation before locking 
it's fp and to sfp_unlock the moment we know there is no refill 
required.  As an aside, the fwalk code should not be locking any fp and 
letting the operation determine if locking is required (e.g. the 
operation might be a read-only operation that doesn't affect the fp itself).

-- Jeff J.

Andre Heider wrote:
> Hi list,
>
> i have the following situation:
>
> - 2 threads
> - each thread has its own, opened FILE*
> - buffering has been disabled for both via setvbuf
>
> Two concurrent fread() calls can now result in a deadlock:
>
> thread 1:
> #5  0x80014bc4 in __sfp_lock_acquire ()
> #6  0x80016a40 in _fwalk (ptr=<value optimized out>, function=<value 
> optimized out>)
> #7  0x8001b9ac in __srefill_r (ptr=0x80c5ade8, fp=0x80bec394)
> #8  0x80015460 in _fread_r (ptr=0x80c5ade8, buf=<value optimized out>,
>     size=1, count=1024, fp=0x80bec394)
>
> thread 2:
> #5  0x80029a94 in __flockfile (fp=<value optimized out>)
> #6  0x80016ac0 in _fwalk (ptr=<value optimized out>, 
> function=0x8001ba20 <lflush>)
> #7  0x8001b9ac in __srefill_r (ptr=0x807d2520, fp=0x80900e58)
> #8  0x80015460 in _fread_r (ptr=0x807d2520, buf=<value optimized out>, 
> size=1, count=1, fp=0x80900e58)
>
>
> refill.c contains the following comment just above the _fwalk call:
>
>   /*
>    * Before reading from a line buffered or unbuffered file,
>    * flush all line buffered output files, per the ANSI C
>    * standard.
>    */
>
> But it doesn't do that, it flushes every file, no matter what 
> fp->_flags is set to (well, except zero). Attached you'll find a patch 
> which adds a new argument to the fwalk calls (a simple bitmask which 
> is compared to fp->_flags). __srefill_r() now simply passes __SLBF 
> there. I'm not sure if this is the best way to resolve this (i'm new 
> to newlib), i just synced the code with the comment ;)
>
> This patch is against 1.16.0, but it seems CVS has the same issue.
>
> Regards,
> Andre