diff mbox series

newlib: fclose: Use sfp lock while fp lock is active

Message ID 20250716144219.12884-1-takashi.yano@nifty.ne.jp
State New
Headers
Series newlib: fclose: Use sfp lock while fp lock is active |

Commit Message

Takashi Yano July 16, 2025, 2:42 p.m. UTC 
  With the commit 656df313e08a, if a thread acquires sfp lock after
another thread calls fclose() and fp lock is acquired, the first
thread falls into deadlock if it trys to acquire fp lock. This can
happen if the first thread calls __sft_lock_all() while the second
thread calls fclose().

This patch reverts the changes for newlib/libc/stdio/fclose.c in
the commit 656df313e08a.

Addresses: https://cygwin.com/pipermail/cygwin/2025-June/258323.html
Fixes: 656df313e08a ("* libc/stdio/fclose.c: Only use sfp lock to guard non-atomic changes of flags and fp lock.")
Reported-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Reviewed-by:
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 newlib/libc/stdio/fclose.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Corinna Vinschen July 16, 2025, 2:54 p.m. UTC | #1 
On Jul 16 23:42, Takashi Yano via Newlib wrote:
> With the commit 656df313e08a, if a thread acquires sfp lock after
> another thread calls fclose() and fp lock is acquired, the first
> thread falls into deadlock if it trys to acquire fp lock. This can
                                   ^^^^
                                   tries

> happen if the first thread calls __sft_lock_all() while the second
                                     ^^^
                                     sfp

> thread calls fclose().
> 
> This patch reverts the changes for newlib/libc/stdio/fclose.c in
> the commit 656df313e08a.

D'uh, thanks for tracking this down.  That was an optimization attempt
at its worst.  Please push (to 3.6 branch as well, obnviously).


Thanks,
Corinna
diff mbox series

Patch

diff --git a/newlib/libc/stdio/fclose.c b/newlib/libc/stdio/fclose.c
index 983ae2cc4..4c81cbf4b 100644
--- a/newlib/libc/stdio/fclose.c
+++ b/newlib/libc/stdio/fclose.c
@@ -72,6 +72,7 @@  _fclose_r (struct _reent *rptr,
   int __oldcancel;
   pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &__oldcancel);
 #endif
+  __sfp_lock_acquire ();
   if (!(fp->_flags2 & __SNLK))
     _flockfile (fp);
 
@@ -79,6 +80,7 @@  _fclose_r (struct _reent *rptr,
     {
       if (!(fp->_flags2 & __SNLK))
 	_funlockfile (fp);
+      __sfp_lock_release ();
 #ifdef _STDIO_WITH_THREAD_CANCELLATION_SUPPORT
       pthread_setcancelstate (__oldcancel, &__oldcancel);
 #endif
@@ -101,7 +103,6 @@  _fclose_r (struct _reent *rptr,
     FREEUB (rptr, fp);
   if (HASLB (fp))
     FREELB (rptr, fp);
-  __sfp_lock_acquire ();
   fp->_flags = 0;		/* release this FILE for reuse */
   if (!(fp->_flags2 & __SNLK))
     _funlockfile (fp);