Message ID | f6757145-6d1a-e1e5-6b2b-9f7b7e65e26b@alibaba-inc.com |
---|---|
State | New |
Headers | show |
Series | [RFC] libio: Fix deadlock between freopen and fclose [BZ #24963] | expand |
On 21/09/2019 04:21, 谢宜生(毅晟) wrote: > we find a deadlock between fclose and freopen as following: > CPU0 CPU1 > freopen fclose > ->_IO_acquire_lock (fp) ->_IO_un_link > ->_IO_file_close_it ->_IO_lock_lock(list_all_lock) > ->_IO_un_link > ->_IO_lock_lock (list_all_lock)<-wait here > ->_IO_flockfile((_IO_FILE *) fp); <-- wait here > > As Carlos pointed that this maybe the bug of in _IO_new_fclose, which > can be fixed by locking fp first, then with fp acquired lock the whole > list. if we want to keep freopen as is then an fp cannot be locked after list_all_lock is held, if another thread might freopen(fp). this affects more than just fclose, e.g. fflush(NULL) would have the same lock ordering (first locks list_all_lock, then locks each fp) this tells me that a different fix is needed, either way there should be a bug report about it in bugzilla (and the id referenced in the changelog of a patch submission assuming you have the fsf copyright assignment sorted out). > > Signed-off-by: Yisheng Xie <yisheng.xys@alibaba-inc.com> > --- > libio/iofclose.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libio/iofclose.c b/libio/iofclose.c > index 398b86d597..fe262cf6aa 100644 > --- a/libio/iofclose.c > +++ b/libio/iofclose.c > @@ -44,11 +44,11 @@ _IO_new_fclose (FILE *fp) > return _IO_old_fclose (fp); > #endif > > + _IO_acquire_lock (fp); > /* First unlink the stream. */ > if (fp->_flags & _IO_IS_FILEBUF) > _IO_un_link ((struct _IO_FILE_plus *) fp); > > - _IO_acquire_lock (fp); > if (fp->_flags & _IO_IS_FILEBUF) > status = _IO_file_close_it (fp); > else > -- > 2.23.0 >
On 23/09/2019 11:05, Szabolcs Nagy wrote: > either way there should be a bug report about > it in bugzilla (and the id referenced in the sorry, missed the bugzilla id in the subject, i commented on the bug now.
diff --git a/libio/iofclose.c b/libio/iofclose.c index 398b86d597..fe262cf6aa 100644 --- a/libio/iofclose.c +++ b/libio/iofclose.c @@ -44,11 +44,11 @@ _IO_new_fclose (FILE *fp) return _IO_old_fclose (fp); #endif + _IO_acquire_lock (fp); /* First unlink the stream. */ if (fp->_flags & _IO_IS_FILEBUF) _IO_un_link ((struct _IO_FILE_plus *) fp); - _IO_acquire_lock (fp); if (fp->_flags & _IO_IS_FILEBUF) status = _IO_file_close_it (fp); else
we find a deadlock between fclose and freopen as following: CPU0 CPU1 freopen fclose ->_IO_acquire_lock (fp) ->_IO_un_link ->_IO_file_close_it ->_IO_lock_lock(list_all_lock) ->_IO_un_link ->_IO_lock_lock (list_all_lock)<-wait here ->_IO_flockfile((_IO_FILE *) fp); <-- wait here As Carlos pointed that this maybe the bug of in _IO_new_fclose, which can be fixed by locking fp first, then with fp acquired lock the whole list. Signed-off-by: Yisheng Xie <yisheng.xys@alibaba-inc.com> --- libio/iofclose.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.23.0