[v2] Resolve-flockfile-funlockfile-differences

Message ID c4f64515-fec4-24dc-47a2-76b091718530@oracle.com
State Under Review
Delegated to: Florian Weimer
Series [v2] Resolve-flockfile-funlockfile-differences |


Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Patrick McGehearty Nov. 23, 2022, 9:08 p.m. UTC
  From a0291671db0d92a8762de3a45fd322e471a19a24 Mon Sep 17 00:00:00 2001
From: Patrick McGehearty <patrick.mcgehearty@oracle.com>
Date: Wed, 23 Nov 2022 21:02:02 +0000
Subject: [PATCH v2] Resolve flockfile/funlockfile differences

- - - - - -
Only difference from v1 is to correct indenting/tabs
for the changes to match other .c files in stdio-common.
- - - - - -
This patch resolves differences between flockfile/funlockfile and
LOCK_STREAM/UNLOCKSTREAM to avoid failure to unlock a stream mutex in
a multi-threaded application context which allows entering using
LOCK_STREAM but leaving using funlockfile or vice-versa.
This issue was detected during stress tests of a large proprietary
application. The cause and solution was identified by Gerd Rausch.

The issue occurs because _IO_funlockfile has different definitions in
different contexts:

Comparing the inline version in libio/libio.h
#  define _IO_flockfile(_fp) \
  if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_flockfile (_fp)

And the non-inline version in stdio-common/flockfile.c
__flockfile (FILE *stream)
  _IO_lock_lock (*stream->_lock);

Note the lack of the _IO_USER_LOCK in the __flockfile version.  This
difference means it is possible to bypass the lock in some cases and
not release the lock in other cases. Either way, it causes trouble.

The proposed fix is to simple add the _IO_USER_LOCK guard to the
non-line versions of flockfile and funlockfile.

Modified files:
 stdio-common/flockfile.c   | 3 ++-
 stdio-common/funlockfile.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)


diff --git a/stdio-common/flockfile.c b/stdio-common/flockfile.c
index 49f72c6..7c82847 100644
--- a/stdio-common/flockfile.c
+++ b/stdio-common/flockfile.c
@@ -22,7 +22,8 @@ 
 __flockfile (FILE *stream)
-  _IO_lock_lock (*stream->_lock);
+  if ((stream->_flags & _IO_USER_LOCK) == 0)
+    _IO_lock_lock (*stream->_lock);
 weak_alias (__flockfile, flockfile);
 weak_alias (__flockfile, _IO_flockfile)
diff --git a/stdio-common/funlockfile.c b/stdio-common/funlockfile.c
index bf44c99..b77b1b2 100644
--- a/stdio-common/funlockfile.c
+++ b/stdio-common/funlockfile.c
@@ -23,7 +23,8 @@ 
 __funlockfile (FILE *stream)
-  _IO_lock_unlock (*stream->_lock);
+  if ((stream->_flags & _IO_USER_LOCK) == 0)
+    _IO_lock_unlock (*stream->_lock);
 weak_alias (__funlockfile, _IO_funlockfile)
 weak_alias (__funlockfile, funlockfile);