* libc/stdio/fclose.c: Only use sfp lock to guard non-atomic

changes of flags and fp lock.
	* libc/stdio/freopen.c: Ditto.
	* libc/stdio/freopen64.c: Ditto.
	* libc/stdio/fgetc.c: Revert change from 2009-04-24, remove sfp locks
	which guard entire function to avoid potential deadlocks when using
	stdio functions in multiple thraeds.
	* libc/stdio/fgets.c: Ditto.
	* libc/stdio/fgetwc.c: Ditto.
	* libc/stdio/fgetws.c: Ditto.
	* libc/stdio/fread.c: Ditto.
	* libc/stdio/fseek.c: Ditto.
	* libc/stdio/getc.c: Ditto.
	* libc/stdio/getdelim.c: Ditto.
	* libc/stdio/gets.c: Ditto.
	* libc/stdio/vfscanf.c: Ditto.
	* libc/stdio/vfwscanf.c: Ditto.

	* libc/stdio/fflush.c (_fflush_r): Split out core functionality into
	new function __sflush_r.  Just lock file and call __sflush_r from here.
	* libc/stdio/fwalk.c (_fwalk): Remove static helper function and move
	functionality back into main function. Don't walk a file with flags
	value of 1.  Add comment.
	(_fwalk_reent): Ditto.
	* libc/stdio/local.h (__sflush_r): Declare.
	* libc/stdio/refill.c (__srefill): Before calling fwalk, set flags
	value to 1 so this file pointer isn't walked.  Revert flags afterwards
	and call __sflush_r for this fp if necessary.  Add comments.
This commit is contained in:
Corinna Vinschen 2011-01-28 10:49:11 +00:00
parent b5ca0d7271
commit 656df313e0
19 changed files with 106 additions and 132 deletions

View File

@ -1,3 +1,34 @@
2011-01-28 Corinna Vinschen <vinschen@redhat.com>
* libc/stdio/fclose.c: Only use sfp lock to guard non-atomic
changes of flags and fp lock.
* libc/stdio/freopen.c: Ditto.
* libc/stdio/freopen64.c: Ditto.
* libc/stdio/fgetc.c: Revert change from 2009-04-24, remove sfp locks
which guard entire function to avoid potential deadlocks when using
stdio functions in multiple thraeds.
* libc/stdio/fgets.c: Ditto.
* libc/stdio/fgetwc.c: Ditto.
* libc/stdio/fgetws.c: Ditto.
* libc/stdio/fread.c: Ditto.
* libc/stdio/fseek.c: Ditto.
* libc/stdio/getc.c: Ditto.
* libc/stdio/getdelim.c: Ditto.
* libc/stdio/gets.c: Ditto.
* libc/stdio/vfscanf.c: Ditto.
* libc/stdio/vfwscanf.c: Ditto.
* libc/stdio/fflush.c (_fflush_r): Split out core functionality into
new function __sflush_r. Just lock file and call __sflush_r from here.
* libc/stdio/fwalk.c (_fwalk): Remove static helper function and move
functionality back into main function. Don't walk a file with flags
value of 1. Add comment.
(_fwalk_reent): Ditto.
* libc/stdio/local.h (__sflush_r): Declare.
* libc/stdio/refill.c (__srefill): Before calling fwalk, set flags
value to 1 so this file pointer isn't walked. Revert flags afterwards
and call __sflush_r for this fp if necessary. Add comments.
2011-01-27 Corinna Vinschen <vinschen@redhat.com>
* libc/include/sys/features.h: Define __STDC_ISO_10646__ for Cygwin.

View File

@ -74,8 +74,6 @@ _DEFUN(_fclose_r, (rptr, fp),
if (fp == NULL)
return (0); /* on NULL */
__sfp_lock_acquire ();
CHECK_INIT (rptr, fp);
_flockfile (fp);
@ -83,7 +81,6 @@ _DEFUN(_fclose_r, (rptr, fp),
if (fp->_flags == 0) /* not open! */
{
_funlockfile (fp);
__sfp_lock_release ();
return (0);
}
/* Unconditionally flush to allow special handling for seekable read
@ -98,6 +95,7 @@ _DEFUN(_fclose_r, (rptr, fp),
FREEUB (rptr, fp);
if (HASLB (fp))
FREELB (rptr, fp);
__sfp_lock_acquire ();
fp->_flags = 0; /* release this FILE for reuse */
_funlockfile (fp);
#ifndef __SINGLE_THREAD__

View File

@ -67,37 +67,16 @@ No supporting OS subroutines are required.
/* Flush a single file, or (if fp is NULL) all files. */
/* Core function which does not lock file pointer. This gets called
directly from __srefill. */
int
_DEFUN(_fflush_r, (ptr, fp),
_DEFUN(__sflush_r, (ptr, fp),
struct _reent *ptr _AND
register FILE * fp)
{
register unsigned char *p;
register int n, t;
#ifdef _REENT_SMALL
/* For REENT_SMALL platforms, it is possible we are being
called for the first time on a std stream. This std
stream can belong to a reentrant struct that is not
_REENT. If CHECK_INIT gets called below based on _REENT,
we will end up changing said file pointers to the equivalent
std stream off of _REENT. This causes unexpected behavior if
there is any data to flush on the _REENT std stream. There
are two alternatives to fix this: 1) make a reentrant fflush
or 2) simply recognize that this file has nothing to flush
and return immediately before performing a CHECK_INIT. Choice
2 is implemented here due to its simplicity. */
if (fp->_bf._base == NULL)
return 0;
#endif /* _REENT_SMALL */
CHECK_INIT (ptr, fp);
if (!fp->_flags)
return 0;
_flockfile (fp);
t = fp->_flags;
if ((t & __SWR) == 0)
{
@ -150,7 +129,6 @@ _DEFUN(_fflush_r, (ptr, fp),
}
else
fp->_flags |= __SERR;
_funlockfile (fp);
return result;
}
}
@ -186,17 +164,14 @@ _DEFUN(_fflush_r, (ptr, fp),
else
{
fp->_flags |= __SERR;
_funlockfile (fp);
return EOF;
}
}
_funlockfile (fp);
return 0;
}
if ((p = fp->_bf._base) == NULL)
{
/* Nothing to flush. */
_funlockfile (fp);
return 0;
}
n = fp->_p - p; /* write this much */
@ -215,16 +190,48 @@ _DEFUN(_fflush_r, (ptr, fp),
if (t <= 0)
{
fp->_flags |= __SERR;
_funlockfile (fp);
return EOF;
}
p += t;
n -= t;
}
_funlockfile (fp);
return 0;
}
int
_DEFUN(_fflush_r, (ptr, fp),
struct _reent *ptr _AND
register FILE * fp)
{
int ret;
#ifdef _REENT_SMALL
/* For REENT_SMALL platforms, it is possible we are being
called for the first time on a std stream. This std
stream can belong to a reentrant struct that is not
_REENT. If CHECK_INIT gets called below based on _REENT,
we will end up changing said file pointers to the equivalent
std stream off of _REENT. This causes unexpected behavior if
there is any data to flush on the _REENT std stream. There
are two alternatives to fix this: 1) make a reentrant fflush
or 2) simply recognize that this file has nothing to flush
and return immediately before performing a CHECK_INIT. Choice
2 is implemented here due to its simplicity. */
if (fp->_bf._base == NULL)
return 0;
#endif /* _REENT_SMALL */
CHECK_INIT (ptr, fp);
if (!fp->_flags)
return 0;
_flockfile (fp);
ret = __sflush_r (ptr, fp);
_funlockfile (fp);
return ret;
}
#ifndef _REENT_ONLY
int

View File

@ -93,11 +93,9 @@ _DEFUN(fgetc, (fp),
#if !defined(PREFER_SIZE_OVER_SPEED) && !defined(__OPTIMIZE_SIZE__)
int result;
CHECK_INIT(_REENT, fp);
__sfp_lock_acquire ();
_flockfile (fp);
result = __sgetc_r (_REENT, fp);
_funlockfile (fp);
__sfp_lock_release ();
return result;
#else
return _fgetc_r (_REENT, fp);

View File

@ -98,7 +98,6 @@ _DEFUN(_fgets_r, (ptr, buf, n, fp),
CHECK_INIT(ptr, fp);
__sfp_lock_acquire ();
_flockfile (fp);
#ifdef __SCLE
if (fp->_flags & __SCLE)
@ -114,12 +113,10 @@ _DEFUN(_fgets_r, (ptr, buf, n, fp),
if (c == EOF && s == buf)
{
_funlockfile (fp);
__sfp_lock_release ();
return NULL;
}
*s = 0;
_funlockfile (fp);
__sfp_lock_release ();
return buf;
}
#endif
@ -138,7 +135,6 @@ _DEFUN(_fgets_r, (ptr, buf, n, fp),
if (s == buf)
{
_funlockfile (fp);
__sfp_lock_release ();
return 0;
}
break;
@ -164,7 +160,6 @@ _DEFUN(_fgets_r, (ptr, buf, n, fp),
_CAST_VOID memcpy ((_PTR) s, (_PTR) p, len);
s[len] = 0;
_funlockfile (fp);
__sfp_lock_release ();
return (buf);
}
fp->_r -= len;
@ -175,7 +170,6 @@ _DEFUN(_fgets_r, (ptr, buf, n, fp),
while ((n -= len) != 0);
*s = 0;
_funlockfile (fp);
__sfp_lock_release ();
return buf;
}

View File

@ -164,12 +164,10 @@ _DEFUN(_fgetwc_r, (ptr, fp),
{
wint_t r;
__sfp_lock_acquire ();
_flockfile (fp);
ORIENT(fp, 1);
r = __fgetwc (ptr, fp);
_funlockfile (fp);
__sfp_lock_release ();
return r;
}

View File

@ -93,7 +93,6 @@ _DEFUN(_fgetws_r, (ptr, ws, n, fp),
const char *src;
unsigned char *nl;
__sfp_lock_acquire ();
_flockfile (fp);
ORIENT (fp, 1);
@ -144,12 +143,10 @@ _DEFUN(_fgetws_r, (ptr, ws, n, fp),
goto error;
*wsp++ = L'\0';
_funlockfile (fp);
__sfp_lock_release ();
return ws;
error:
_funlockfile (fp);
__sfp_lock_release ();
return NULL;
}

View File

@ -146,7 +146,6 @@ _DEFUN(_fread_r, (ptr, buf, size, count, fp),
CHECK_INIT(ptr, fp);
__sfp_lock_acquire ();
_flockfile (fp);
ORIENT (fp, -1);
if (fp->_r < 0)
@ -197,12 +196,10 @@ _DEFUN(_fread_r, (ptr, buf, size, count, fp),
if (fp->_flags & __SCLE)
{
_funlockfile (fp);
__sfp_lock_release ();
return crlf_r (ptr, fp, buf, total-resid, 1) / size;
}
#endif
_funlockfile (fp);
__sfp_lock_release ();
return (total - resid) / size;
}
}
@ -224,12 +221,10 @@ _DEFUN(_fread_r, (ptr, buf, size, count, fp),
if (fp->_flags & __SCLE)
{
_funlockfile (fp);
__sfp_lock_release ();
return crlf_r (ptr, fp, buf, total-resid, 1) / size;
}
#endif
_funlockfile (fp);
__sfp_lock_release ();
return (total - resid) / size;
}
}
@ -243,12 +238,10 @@ _DEFUN(_fread_r, (ptr, buf, size, count, fp),
if (fp->_flags & __SCLE)
{
_funlockfile (fp);
__sfp_lock_release ();
return crlf_r(ptr, fp, buf, total, 0) / size;
}
#endif
_funlockfile (fp);
__sfp_lock_release ();
return count;
}

View File

@ -98,8 +98,6 @@ _DEFUN(_freopen_r, (ptr, file, mode, fp),
int flags, oflags;
int e = 0;
__sfp_lock_acquire ();
CHECK_INIT (ptr, fp);
_flockfile (fp);
@ -108,7 +106,6 @@ _DEFUN(_freopen_r, (ptr, file, mode, fp),
{
_funlockfile (fp);
_fclose_r (ptr, fp);
__sfp_lock_release ();
return NULL;
}
@ -208,6 +205,7 @@ _DEFUN(_freopen_r, (ptr, file, mode, fp),
if (f < 0)
{ /* did not get it after all */
__sfp_lock_acquire ();
fp->_flags = 0; /* set it free */
ptr->_errno = e; /* restore in case _close clobbered */
_funlockfile (fp);
@ -232,7 +230,6 @@ _DEFUN(_freopen_r, (ptr, file, mode, fp),
#endif
_funlockfile (fp);
__sfp_lock_release ();
return fp;
}

View File

@ -138,7 +138,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
CHECK_INIT (ptr, fp);
__sfp_lock_acquire ();
_flockfile (fp);
/* If we've been doing some writing, and we're in append mode
@ -156,7 +155,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
{
ptr->_errno = ESPIPE; /* ??? */
_funlockfile (fp);
__sfp_lock_release ();
return EOF;
}
@ -182,7 +180,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
if (curoff == -1L)
{
_funlockfile (fp);
__sfp_lock_release ();
return EOF;
}
}
@ -208,7 +205,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
default:
ptr->_errno = EINVAL;
_funlockfile (fp);
__sfp_lock_release ();
return (EOF);
}
@ -268,7 +264,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
{
ptr->_errno = EOVERFLOW;
_funlockfile (fp);
__sfp_lock_release ();
return EOF;
}
@ -325,7 +320,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
fp->_flags &= ~__SEOF;
memset (&fp->_mbstate, 0, sizeof (_mbstate_t));
_funlockfile (fp);
__sfp_lock_release ();
return 0;
}
@ -356,7 +350,6 @@ _DEFUN(_fseek_r, (ptr, fp, offset, whence),
}
memset (&fp->_mbstate, 0, sizeof (_mbstate_t));
_funlockfile (fp);
__sfp_lock_release ();
return 0;
/*
@ -369,7 +362,6 @@ dumb:
|| seekfn (ptr, fp->_cookie, offset, whence) == POS_ERR)
{
_funlockfile (fp);
__sfp_lock_release ();
return EOF;
}
/* success: clear EOF indicator and discard ungetc() data */
@ -388,7 +380,6 @@ dumb:
fp->_flags &= ~__SNPT;
memset (&fp->_mbstate, 0, sizeof (_mbstate_t));
_funlockfile (fp);
__sfp_lock_release ();
return 0;
}

View File

@ -27,8 +27,8 @@ static char sccsid[] = "%W% (Berkeley) %G%";
#include <errno.h>
#include "local.h"
static int
_DEFUN(__fwalk, (ptr, function),
int
_DEFUN(_fwalk, (ptr, function),
struct _reent *ptr _AND
register int (*function) (FILE *))
{
@ -36,11 +36,19 @@ _DEFUN(__fwalk, (ptr, function),
register int n, ret = 0;
register struct _glue *g;
/*
* It should be safe to walk the list without locking it;
* new nodes are only added to the end and none are ever
* removed.
*
* Avoid locking this list while walking it or else you will
* introduce a potential deadlock in [at least] refill.c.
*/
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->_file != -1)
if (fp->_flags != 0 && fp->_flags != 1 && fp->_file != -1)
ret |= (*function) (fp);
}
@ -49,8 +57,8 @@ _DEFUN(__fwalk, (ptr, function),
/* Special version of __fwalk where the function pointer is a reentrant
I/O function (e.g. _fclose_r). */
static int
_DEFUN(__fwalk_reent, (ptr, reent_function),
int
_DEFUN(_fwalk_reent, (ptr, reent_function),
struct _reent *ptr _AND
register int (*reent_function) (struct _reent *, FILE *))
{
@ -58,51 +66,21 @@ _DEFUN(__fwalk_reent, (ptr, reent_function),
register int n, ret = 0;
register struct _glue *g;
/*
* It should be safe to walk the list without locking it;
* new nodes are only added to the end and none are ever
* removed.
*
* Avoid locking this list while walking it or else you will
* introduce a potential deadlock in [at least] refill.c.
*/
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->_file != -1)
if (fp->_flags != 0 && fp->_flags != 1 && fp->_file != -1)
ret |= (*reent_function) (ptr, fp);
}
return ret;
}
int
_DEFUN(_fwalk, (ptr, function),
struct _reent *ptr _AND
register int (*function)(FILE *))
{
register int ret = 0;
__sfp_lock_acquire ();
/* Must traverse given list for streams. Note that _GLOBAL_REENT
only walked once in exit(). */
ret |= __fwalk (ptr, function);
__sfp_lock_release ();
return ret;
}
/* Special version of _fwalk which handles a function pointer to a
reentrant I/O function (e.g. _fclose_r). */
int
_DEFUN(_fwalk_reent, (ptr, reent_function),
struct _reent *ptr _AND
register int (*reent_function) (struct _reent *, FILE *))
{
register int ret = 0;
__sfp_lock_acquire ();
/* Must traverse given list for streams. Note that _GLOBAL_REENT
only walked once in exit(). */
ret |= __fwalk_reent (ptr, reent_function);
__sfp_lock_release ();
return ret;
}

View File

@ -92,11 +92,9 @@ _DEFUN(_getc_r, (ptr, fp),
{
int result;
CHECK_INIT (ptr, fp);
__sfp_lock_acquire ();
_flockfile (fp);
result = __sgetc_r (ptr, fp);
_funlockfile (fp);
__sfp_lock_release ();
return result;
}
@ -108,11 +106,9 @@ _DEFUN(getc, (fp),
{
int result;
CHECK_INIT (_REENT, fp);
__sfp_lock_acquire ();
_flockfile (fp);
result = __sgetc_r (_REENT, fp);
_funlockfile (fp);
__sfp_lock_release ();
return result;
}

View File

@ -81,7 +81,6 @@ _DEFUN(__getdelim, (bufptr, n, delim, fp),
CHECK_INIT (_REENT, fp);
__sfp_lock_acquire ();
_flockfile (fp);
numbytes = *n;
@ -131,7 +130,6 @@ _DEFUN(__getdelim, (bufptr, n, delim, fp),
}
_funlockfile (fp);
__sfp_lock_release ();
/* if no input data, return failure */
if (ptr == buf)

View File

@ -79,14 +79,12 @@ _DEFUN(_gets_r, (ptr, buf),
register int c;
register char *s = buf;
__sfp_lock_acquire ();
_flockfile (stdin);
while ((c = __sgetc_r (ptr, stdin)) != '\n')
if (c == EOF)
if (s == buf)
{
_funlockfile (stdin);
__sfp_lock_release ();
return NULL;
}
else
@ -95,7 +93,6 @@ _DEFUN(_gets_r, (ptr, buf),
*s++ = c;
*s = 0;
_funlockfile (stdin);
__sfp_lock_release ();
return buf;
}

View File

@ -54,6 +54,7 @@ int _EXFUN(_svfiwprintf_r,(struct _reent *, FILE *, const wchar_t *,
va_list));
extern FILE *_EXFUN(__sfp,(struct _reent *));
extern int _EXFUN(__sflags,(struct _reent *,_CONST char*, int*));
extern int _EXFUN(__sflush_r,(struct _reent *,FILE *));
extern int _EXFUN(__srefill_r,(struct _reent *,FILE *));
extern _READ_WRITE_RETURN_TYPE _EXFUN(__sread,(struct _reent *, void *, char *,
int));

View File

@ -102,9 +102,19 @@ _DEFUN(__srefill_r, (ptr, fp),
* flush all line buffered output files, per the ANSI C
* standard.
*/
if (fp->_flags & (__SLBF | __SNBF))
_CAST_VOID _fwalk (_GLOBAL_REENT, lflush);
{
/* Ignore this file in _fwalk to avoid potential deadlock. */
short orig_flags = fp->_flags;
fp->_flags = 1;
_CAST_VOID _fwalk (_GLOBAL_REENT, lflush);
fp->_flags = orig_flags;
/* Now flush this file without locking it. */
if ((fp->_flags & (__SLBF|__SWR)) == (__SLBF|__SWR))
__sflush_r (ptr, fp);
}
fp->_p = fp->_bf._base;
fp->_r = fp->_read (ptr, fp->_cookie, (char *) fp->_p, fp->_bf._size);
#ifndef __CYGWIN__

View File

@ -494,7 +494,6 @@ _DEFUN(__SVFSCANF_R, (rptr, fp, fmt0, ap),
# define GET_ARG(n, ap, type) (va_arg (ap, type))
#endif
__sfp_lock_acquire ();
_flockfile (fp);
ORIENT (fp, -1);
@ -795,7 +794,6 @@ _DEFUN(__SVFSCANF_R, (rptr, fp, fmt0, ap),
*/
case '\0': /* compat */
_funlockfile (fp);
__sfp_lock_release ();
return EOF;
default: /* compat */
@ -1596,13 +1594,11 @@ input_failure:
invalid format string), return EOF if no matches yet, else number
of matches made prior to failure. */
_funlockfile (fp);
__sfp_lock_release ();
return nassigned && !(fp->_flags & __SERR) ? nassigned : EOF;
match_failure:
all_done:
/* Return number of matches, which can be 0 on match failure. */
_funlockfile (fp);
__sfp_lock_release ();
return nassigned;
}

View File

@ -434,7 +434,6 @@ _DEFUN(__SVFWSCANF_R, (rptr, fp, fmt0, ap),
# define GET_ARG(n, ap, type) (va_arg (ap, type))
#endif
__sfp_lock_acquire ();
_flockfile (fp);
ORIENT (fp, 1);
@ -714,7 +713,6 @@ _DEFUN(__SVFWSCANF_R, (rptr, fp, fmt0, ap),
*/
case L'\0': /* compat */
_funlockfile (fp);
__sfp_lock_release ();
return EOF;
default: /* compat */
@ -1443,13 +1441,11 @@ input_failure:
invalid format string), return EOF if no matches yet, else number
of matches made prior to failure. */
_funlockfile (fp);
__sfp_lock_release ();
return nassigned && !(fp->_flags & __SERR) ? nassigned : EOF;
match_failure:
all_done:
/* Return number of matches, which can be 0 on match failure. */
_funlockfile (fp);
__sfp_lock_release ();
return nassigned;
}

View File

@ -97,7 +97,6 @@ _DEFUN (_freopen64_r, (ptr, file, mode, fp),
int flags, oflags;
int e = 0;
__sfp_lock_acquire ();
CHECK_INIT (ptr, fp);
@ -107,7 +106,6 @@ _DEFUN (_freopen64_r, (ptr, file, mode, fp),
{
_funlockfile(fp);
_fclose_r (ptr, fp);
__sfp_lock_release ();
return NULL;
}
@ -204,6 +202,7 @@ _DEFUN (_freopen64_r, (ptr, file, mode, fp),
if (f < 0)
{ /* did not get it after all */
__sfp_lock_acquire ();
fp->_flags = 0; /* set it free */
ptr->_errno = e; /* restore in case _close clobbered */
_funlockfile(fp);
@ -231,7 +230,6 @@ _DEFUN (_freopen64_r, (ptr, file, mode, fp),
fp->_flags |= __SL64;
_funlockfile(fp);
__sfp_lock_release ();
return fp;
}