This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
unbuffered fread() deadlock
- From: Andre Heider <a dot heider at gmx dot de>
- To: newlib at sourceware dot org
- Date: Sun, 11 Jan 2009 12:04:22 +0100
- Subject: unbuffered fread() deadlock
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
diff -ur newlib-1.16.0-pre/newlib/libc/stdio/fcloseall.c newlib-1.16.0/newlib/libc/stdio/fcloseall.c
--- newlib-1.16.0-pre/newlib/libc/stdio/fcloseall.c 2009-01-03 01:19:34.000000000 +0100
+++ newlib-1.16.0/newlib/libc/stdio/fcloseall.c 2009-01-11 11:10:18.000000000 +0100
@@ -67,7 +67,7 @@
_DEFUN(_fcloseall_r, (ptr),
struct _reent *ptr)
{
- return _fwalk_reent (ptr, _fclose_r);
+ return _fwalk_reent (ptr, _fclose_r, 0);
}
#ifndef _REENT_ONLY
diff -ur newlib-1.16.0-pre/newlib/libc/stdio/fflush.c newlib-1.16.0/newlib/libc/stdio/fflush.c
--- newlib-1.16.0-pre/newlib/libc/stdio/fflush.c 2009-01-03 01:19:34.000000000 +0100
+++ newlib-1.16.0/newlib/libc/stdio/fflush.c 2009-01-10 22:35:57.000000000 +0100
@@ -214,7 +214,7 @@
register FILE * fp)
{
if (fp == NULL)
- return _fwalk_reent (_GLOBAL_REENT, _fflush_r);
+ return _fwalk_reent (_GLOBAL_REENT, _fflush_r, 0);
return _fflush_r (_REENT, fp);
}
diff -ur newlib-1.16.0-pre/newlib/libc/stdio/findfp.c newlib-1.16.0/newlib/libc/stdio/findfp.c
--- newlib-1.16.0-pre/newlib/libc/stdio/findfp.c 2009-01-03 01:19:34.000000000 +0100
+++ newlib-1.16.0/newlib/libc/stdio/findfp.c 2009-01-10 22:36:36.000000000 +0100
@@ -158,7 +158,7 @@
_DEFUN(_cleanup_r, (ptr),
struct _reent *ptr)
{
- _CAST_VOID _fwalk(ptr, fclose);
+ _CAST_VOID _fwalk(ptr, fclose, 0);
/* _CAST_VOID _fwalk (ptr, fflush); */ /* `cheating' */
}
@@ -277,13 +278,13 @@
{
__sfp_lock_acquire ();
- _CAST_VOID _fwalk (_REENT, __fp_lock);
+ _CAST_VOID _fwalk (_REENT, __fp_lock, 0);
}
_VOID
_DEFUN_VOID(__fp_unlock_all)
{
- _CAST_VOID _fwalk (_REENT, __fp_unlock);
+ _CAST_VOID _fwalk (_REENT, __fp_unlock, 0);
__sfp_lock_release ();
}
diff -ur newlib-1.16.0-pre/newlib/libc/stdio/fwalk.c newlib-1.16.0/newlib/libc/stdio/fwalk.c
--- newlib-1.16.0-pre/newlib/libc/stdio/fwalk.c 2009-01-03 01:19:34.000000000 +0100
+++ newlib-1.16.0/newlib/libc/stdio/fwalk.c 2009-01-10 23:42:23.000000000 +0100
@@ -30,7 +30,8 @@
static int
_DEFUN(__fwalk, (ptr, function),
struct _reent *ptr _AND
- register int (*function) (FILE *))
+ register int (*function) (FILE *) _AND
+ register short flags)
{
register FILE *fp;
register int n, ret = 0;
@@ -38,7 +39,7 @@
for (g = &ptr->__sglue; g != NULL; g = g->_next)
for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
- if (fp->_flags != 0)
+ if (fp->_flags != 0 && (fp->_flags & flags) == flags)
{
_flockfile (fp);
if (fp->_flags != 0 && fp->_file != -1)
@@ -54,7 +55,8 @@
static int
_DEFUN(__fwalk_reent, (ptr, reent_function),
struct _reent *ptr _AND
- register int (*reent_function) (struct _reent *, FILE *))
+ register int (*reent_function) (struct _reent *, FILE *) _AND
+ register short flags)
{
register FILE *fp;
register int n, ret = 0;
@@ -62,7 +64,7 @@
for (g = &ptr->__sglue; g != NULL; g = g->_next)
for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
- if (fp->_flags != 0)
+ if (fp->_flags != 0 && (fp->_flags & flags) == flags)
{
_flockfile (fp);
if (fp->_flags != 0 && fp->_file != -1)
@@ -76,7 +78,8 @@
int
_DEFUN(_fwalk, (ptr, function),
struct _reent *ptr _AND
- register int (*function)(FILE *))
+ register int (*function)(FILE *) _AND
+ register short flags)
{
register int ret = 0;
@@ -84,7 +87,7 @@
/* Must traverse given list for streams. Note that _GLOBAL_REENT
only walked once in exit(). */
- ret |= __fwalk (ptr, function);
+ ret |= __fwalk (ptr, function, flags);
__sfp_lock_release ();
@@ -96,7 +99,8 @@
int
_DEFUN(_fwalk_reent, (ptr, reent_function),
struct _reent *ptr _AND
- register int (*reent_function) (struct _reent *, FILE *))
+ register int (*reent_function) (struct _reent *, FILE *) _AND
+ register short flags)
{
register int ret = 0;
@@ -104,7 +108,7 @@
/* Must traverse given list for streams. Note that _GLOBAL_REENT
only walked once in exit(). */
- ret |= __fwalk_reent (ptr, reent_function);
+ ret |= __fwalk_reent (ptr, reent_function, flags);
__sfp_lock_release ();
diff -ur newlib-1.16.0-pre/newlib/libc/stdio/local.h newlib-1.16.0/newlib/libc/stdio/local.h
--- newlib-1.16.0-pre/newlib/libc/stdio/local.h 2009-01-03 01:19:34.000000000 +0100
+++ newlib-1.16.0/newlib/libc/stdio/local.h 2009-01-10 22:35:28.000000000 +0100
@@ -48,8 +48,8 @@
extern _VOID _EXFUN(__sinit,(struct _reent *));
extern _VOID _EXFUN(_cleanup_r,(struct _reent *));
extern _VOID _EXFUN(__smakebuf_r,(struct _reent *, FILE *));
-extern int _EXFUN(_fwalk,(struct _reent *, int (*)(FILE *)));
-extern int _EXFUN(_fwalk_reent,(struct _reent *, int (*)(struct _reent *, FILE *)));
+extern int _EXFUN(_fwalk,(struct _reent *, int (*)(FILE *), short));
+extern int _EXFUN(_fwalk_reent,(struct _reent *, int (*)(struct _reent *, FILE *), short));
struct _glue * _EXFUN(__sfmoreglue,(struct _reent *,int n));
#ifdef __LARGE64_FILES
diff -ur newlib-1.16.0-pre/newlib/libc/stdio/refill.c newlib-1.16.0/newlib/libc/stdio/refill.c
--- newlib-1.16.0-pre/newlib/libc/stdio/refill.c 2009-01-03 01:19:34.000000000 +0100
+++ newlib-1.16.0/newlib/libc/stdio/refill.c 2009-01-10 22:37:56.000000000 +0100
@@ -102,7 +102,7 @@
*/
if (fp->_flags & (__SLBF | __SNBF))
- _CAST_VOID _fwalk (_GLOBAL_REENT, lflush);
+ _CAST_VOID _fwalk (_GLOBAL_REENT, lflush, __SLBF);
fp->_p = fp->_bf._base;
fp->_r = fp->_read (ptr, fp->_cookie, (char *) fp->_p, fp->_bf._size);
fp->_flags &= ~__SMOD; /* buffer contents are again pristine */