On Fri, Nov 27, 2020 at 10:35 PM Philip Guenther <guent...@gmail.com> wrote:
...
> So yeah, maybe it does work to:
> 1) make __sfp() FLOCKFILE() the allocated FILE before unlocking sfp_mutex
> 2) update f{,d,mem,un}open() and open_*memstream() to match (1), unlocking
> in all paths when the FILE is safe to be accessed by _fwalk(), and
> locking
> sfp_mutex around the zeroing of _flags.
> 3) make fclose() and freopen() also lock sfp_mutex around the zeroing of
> _flags
> (should add an _frelease() to findfp.c that does this dance for (2) and
> (3))
> 4) make _fwalk() take sfp_mutex, and maybe also a FILE* so the setting of
> __SIGN can be done under the lock?
>
5) decide how/whether to handle adjust the FLOCKFILE placement in the
> _fwalk()
> tree: is the testing of the "is line-buffered" flag in lflush() safe
> without
> a lock? Mumble...
>
After thinking through states more, #4 isn't safe: _fwalk() can't take
sfp_mutex because it's called from __srefill() which has its FILE locked,
which would reverse the order: two concurrent calls to __srefill() from
line-buffered FILEs could have one in _fwalk() blocking on locking the
other, while the other blocks on the sfp_mutex for _fwalk().
Hmm, there's currently a loop between two __srefill() calls like that, as
there's nothing to force visibility of the __SIGN flag between CPUs so they
could try to lock each other. Grrr.
Time to check other BSDs and see if they have a good solution to this...
Philip
>
> Now, back to that first assumption: if you're not willing to assume
> it then the "is allocated" test needs to not use _flags but be some
> other state change which can be relied on to not have false-positives,
> but otherwise the other bits above still apply, I believe. Would
> be a major ABI change...and if we do that there's like 3 other
> things we should do at the same time (merge __sfileext into FILE,
> grow _file to an int, and maybe move the recursive mutex for
> f{,un}lockfile() into FILE?)
>
>
> Philip Guenther
>
>
