From patchwork Wed Nov 13 11:21:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Schwab X-Patchwork-Id: 35858 Received: (qmail 101225 invoked by alias); 13 Nov 2019 11:21:57 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 101202 invoked by uid 89); 13 Nov 2019 11:21:57 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.suse.de From: Andreas Schwab To: libc-alpha@sourceware.org Subject: [PATCH] Always do locking when accessing streams (bug 15142) X-Yow: -- I can do ANYTHING ... I can even ... SHOPLIFT!! Date: Wed, 13 Nov 2019 12:21:51 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 During exit, skip files that are currently locked to avoid deadlock. --- libio/genops.c | 54 ++++++++++++++++++++++++++------------------------ libio/libio.h | 4 ++++ libio/libioP.h | 1 - 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/libio/genops.c b/libio/genops.c index f871e7751b..30daa169ef 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -681,8 +681,8 @@ _IO_adjust_column (unsigned start, const char *line, int count) } libc_hidden_def (_IO_adjust_column) -int -_IO_flush_all_lockp (int do_lock) +static int +_IO_flush_all_lockp (bool skip_locked) { int result = 0; FILE *fp; @@ -695,7 +695,16 @@ _IO_flush_all_lockp (int do_lock) for (fp = (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) @@ -706,8 +715,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; } @@ -724,7 +732,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) @@ -789,6 +797,14 @@ _IO_unbuffer_all (void) for (fp = (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; + } + int legacy = 0; #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) @@ -800,18 +816,6 @@ _IO_unbuffer_all (void) /* Iff stream is un-orientated, it wasn't used. */ && (legacy || 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 (! legacy && ! dealloc_buffers && !(fp->_flags & _IO_USER_BUF)) { fp->_flags |= _IO_USER_BUF; @@ -825,17 +829,15 @@ _IO_unbuffer_all (void) if (! legacy && 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. */ if (! legacy) fp->_mode = -1; + + _IO_funlockfile (fp); + run_fp = NULL; } #ifdef _IO_MTSAFE_IO @@ -861,9 +863,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/libio.h b/libio/libio.h index bed324e57d..3e660bd454 100644 --- a/libio/libio.h +++ b/libio/libio.h @@ -291,11 +291,15 @@ libc_hidden_proto (_IO_sgetn) 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/libioP.h b/libio/libioP.h index 3787605cfb..2187b8edbe 100644 --- a/libio/libioP.h +++ b/libio/libioP.h @@ -487,7 +487,6 @@ extern int _IO_new_do_write (FILE *, const char *, size_t); extern int _IO_old_do_write (FILE *, const char *, size_t); extern int _IO_wdo_write (FILE *, const wchar_t *, 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);