Sebastien Marie <[email protected]> wrote:

> On Sat, Mar 02, 2019 at 04:00:19PM +0000, Visa Hankala wrote:
> > On Sat, Mar 02, 2019 at 04:37:04PM +0100, Sebastien Marie wrote:
> > > Thread 1 (thread 469200):
> > > #0  sched_yield () at -:3
> > > #1  0x00000a8c0609d9c5 in _libc__spinlock (lock=Variable "lock" is not 
> > > available.) at /usr/src/lib/libc/thread/rthread.c:50
> > > #2  0x00000a8c060702be in _thread_flockfile (fp=0x7f7ffffcfaa8) at 
> > > /usr/src/lib/libc/thread/rthread_file.c:180
> > > #3  0x00000a8c0609e11a in _libc_fflush (fp=0x7f7ffffcfaa8) at 
> > > /usr/src/lib/libc/stdio/fflush.c:46
> > > #4  0x00000a8c0606c89a in _libc_vdprintf (fd=Variable "fd" is not 
> > > available.) at /usr/src/lib/libc/stdio/vdprintf.c:72
> > > #5  0x00000a8c060a7f63 in _libc__rthread_debug (level=Variable "level" is 
> > > not available.) at /usr/src/lib/libc/thread/rthread_debug.c:23
> > > #6  0x00000a8c060410c5 in _rthread_mutex_timedlock (mutexp=Variable 
> > > "mutexp" is not available.) at 
> > > /usr/src/lib/libc/thread/rthread_mutex.c:163
> > > #7  0x00000a8c060bc482 in malloc (size=56) at 
> > > /usr/src/lib/libc/stdlib/malloc.c:1253
> > > #8  0x00000a8c060703e4 in _thread_flockfile (fp=0x7f7ffffd02b8) at 
> > > /usr/src/lib/libc/thread/rthread_file.c:156
> > > #9  0x00000a8c0609e11a in _libc_fflush (fp=0x7f7ffffd02b8) at 
> > > /usr/src/lib/libc/stdio/fflush.c:46
> > > #10 0x00000a8c0606c89a in _libc_vdprintf (fd=Variable "fd" is not 
> > > available.) at /usr/src/lib/libc/stdio/vdprintf.c:72
> > > #11 0x00000a8c060a7f63 in _libc__rthread_debug (level=Variable "level" is 
> > > not available.) at /usr/src/lib/libc/thread/rthread_debug.c:23
> > > #12 0x00000a8c3c2792b6 in _rthread_reaper () at 
> > > /data/openbsd/src/lib/librthread/rthread.c:260
> > > #13 0x00000a8c3c279229 in pthread_join (thread=Variable "thread" is not 
> > > available.) at /data/openbsd/src/lib/librthread/rthread.c:319
> > > #14 0x00000a8993d1a705 in main (argc=1, argv=0x7f7ffffd0ab8) at test.c:86
> > 
> > This does not look good. The thread is recursing with hash_lock of
> > rthread_file.c. Apparently triggered by the debug output routine.
> 
> does it makes sens to call __sflush() instead of fflush() in vdprintf() ?
> 
> the FILE variable used by vdprintf() is "home made" on the stack, using
> a buffer on the stack too. And as fflush() only enclose __sflush()
> with FILE locking, I don't see the gain to use fflush() here.
> 
> at each call of vdprintf(), the FILE variable is a fresh variable on the
> stack, so there is no possible concurrent access on the variable address.
> 
> and for what I understand of find_lock() and do_lock() in
> libc/thread/rthread_file.c, the lock mecanism is based on FILE address,
> and not on the descriptor used, so locking a stack variable seems
> unnecessary.
> 
> The point where I am unsure is it doesn't correct the underline problem
> if any: why calling _thread_flockfile() could dead-lock. but it makes
> vdprintf() to avoid this unnecessary code path.
> 
> -- 
> Sebastien Marie
> 
> 
> Index: stdio/vdprintf.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/vdprintf.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 vdprintf.c
> --- stdio/vdprintf.c  31 Aug 2015 02:53:57 -0000      1.2
> +++ stdio/vdprintf.c  3 Mar 2019 10:49:59 -0000
> @@ -69,6 +69,6 @@ vdprintf(int fd, const char * __restrict
>       if ((ret = __vfprintf(&f, fmt, ap)) < 0)
>               return ret;
>  
> -     return fflush(&f) ? EOF : ret;
> +     return __sflush(&f) ? EOF : ret;
>  }
>  DEF_WEAK(vdprintf);
> 

That is exactly what I was thinking!

Reply via email to