diff mbox

Always do locking when accessing streams (bug 15142)

Message ID mvm1si7cmh7.fsf@suse.de
State New, archived
Headers

Commit Message

Andreas Schwab Jan. 30, 2018, 4:58 p.m. UTC 
  During exit, skip files that are currently locked to avoid deadlock.

	[BZ #15142]
	* include/bits/libio.h (_IO_ftrylockfile): Define.
	* libio/genops.c (_IO_flush_all_lockp): Make static.  Rename
	argument to skip_locked, callers changed.  Skip files that are
	locked if skip_locked.
	(_IO_unbuffer_all): Lock files before access, but skip locked
	files.
	* libio/libioP.h (_IO_flush_all_lockp): Don't declare.
---
 include/bits/libio.h |  4 ++++
 libio/genops.c       | 54 +++++++++++++++++++++++++++-------------------------
 libio/libioP.h       |  1 -
 3 files changed, 32 insertions(+), 27 deletions(-)
diff mbox

Patch

diff --git a/include/bits/libio.h b/include/bits/libio.h
index 572395d5ff..0743bf5356 100644
--- a/include/bits/libio.h
+++ b/include/bits/libio.h
@@ -33,11 +33,15 @@  libc_hidden_proto (_IO_vfscanf)
   if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_lock (*(_fp)->_lock)
 #  define _IO_funlockfile(_fp) \
   if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_unlock (*(_fp)->_lock)
+#  define _IO_ftrylockfile(_fp) \
+  (((_fp)->_flags & _IO_USER_LOCK) == 0 ? _IO_lock_trylock (*(_fp)->_lock) : 0)
 # else
 #  define _IO_flockfile(_fp) \
   if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_flockfile (_fp)
 #  define _IO_funlockfile(_fp) \
   if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_funlockfile (_fp)
+#  define _IO_ftrylockfile(_fp) \
+  (((_fp)->_flags & _IO_USER_LOCK) == 0 ? _IO_ftrylockfile (_fp) : 0)
 # endif
 #endif /* _IO_MTSAFE_IO */
 
diff --git a/libio/genops.c b/libio/genops.c
index d6f8050669..21ebf3c2df 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -744,8 +744,8 @@  _IO_get_column (_IO_FILE *fp)
 #endif
 
 
-int
-_IO_flush_all_lockp (int do_lock)
+static int
+_IO_flush_all_lockp (bool skip_locked)
 {
   int result = 0;
   struct _IO_FILE *fp;
@@ -758,7 +758,16 @@  _IO_flush_all_lockp (int do_lock)
   for (fp = (_IO_FILE *) _IO_list_all; fp != NULL; fp = fp->_chain)
     {
       run_fp = fp;
-      if (do_lock)
+      if (skip_locked)
+	{
+	  /* Skip files that are currently locked.  */
+	  if (_IO_ftrylockfile (fp))
+	    {
+	      run_fp = NULL;
+	      continue;
+	    }
+	}
+      else
 	_IO_flockfile (fp);
 
       if (((fp->_mode <= 0 && fp->_IO_write_ptr > fp->_IO_write_base)
@@ -769,8 +778,7 @@  _IO_flush_all_lockp (int do_lock)
 	  && _IO_OVERFLOW (fp, EOF) == EOF)
 	result = EOF;
 
-      if (do_lock)
-	_IO_funlockfile (fp);
+      _IO_funlockfile (fp);
       run_fp = NULL;
     }
 
@@ -787,7 +795,7 @@  int
 _IO_flush_all (void)
 {
   /* We want locking.  */
-  return _IO_flush_all_lockp (1);
+  return _IO_flush_all_lockp (false);
 }
 libc_hidden_def (_IO_flush_all)
 
@@ -852,22 +860,18 @@  _IO_unbuffer_all (void)
 
   for (fp = (_IO_FILE *) _IO_list_all; fp; fp = fp->_chain)
     {
+      run_fp = fp;
+      /* Skip files that are currently locked.  */
+      if (_IO_ftrylockfile (fp))
+	{
+	  run_fp = NULL;
+	  continue;
+	}
+
       if (! (fp->_flags & _IO_UNBUFFERED)
 	  /* Iff stream is un-orientated, it wasn't used. */
 	  && fp->_mode != 0)
 	{
-#ifdef _IO_MTSAFE_IO
-	  int cnt;
-#define MAXTRIES 2
-	  for (cnt = 0; cnt < MAXTRIES; ++cnt)
-	    if (fp->_lock == NULL || _IO_lock_trylock (*fp->_lock) == 0)
-	      break;
-	    else
-	      /* Give the other thread time to finish up its use of the
-		 stream.  */
-	      __sched_yield ();
-#endif
-
 	  if (! dealloc_buffers && !(fp->_flags & _IO_USER_BUF))
 	    {
 	      fp->_flags |= _IO_USER_BUF;
@@ -881,16 +885,14 @@  _IO_unbuffer_all (void)
 
 	  if (fp->_mode > 0)
 	    _IO_wsetb (fp, NULL, NULL, 0);
-
-#ifdef _IO_MTSAFE_IO
-	  if (cnt < MAXTRIES && fp->_lock != NULL)
-	    _IO_lock_unlock (*fp->_lock);
-#endif
 	}
 
       /* Make sure that never again the wide char functions can be
 	 used.  */
       fp->_mode = -1;
+
+      _IO_funlockfile (fp);
+      run_fp = NULL;
     }
 
 #ifdef _IO_MTSAFE_IO
@@ -916,9 +918,9 @@  libc_freeres_fn (buffer_free)
 int
 _IO_cleanup (void)
 {
-  /* We do *not* want locking.  Some threads might use streams but
-     that is their problem, we flush them underneath them.  */
-  int result = _IO_flush_all_lockp (0);
+  /* We want to skip locked streams.  Some threads might use streams but
+     that is their problem, we don't flush those.  */
+  int result = _IO_flush_all_lockp (true);
 
   /* We currently don't have a reliable mechanism for making sure that
      C++ static destructors are executed in the correct order.
diff --git a/libio/libioP.h b/libio/libioP.h
index 068ceb2615..732db5d6a8 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -486,7 +486,6 @@  extern int _IO_new_do_write (_IO_FILE *, const char *, _IO_size_t);
 extern int _IO_old_do_write (_IO_FILE *, const char *, _IO_size_t);
 extern int _IO_wdo_write (_IO_FILE *, const wchar_t *, _IO_size_t);
 libc_hidden_proto (_IO_wdo_write)
-extern int _IO_flush_all_lockp (int);
 extern int _IO_flush_all (void);
 libc_hidden_proto (_IO_flush_all)
 extern int _IO_cleanup (void);