exfile / file locking

[Alan DeKok]

On Jul 25, 2017, at 7:34 AM, Phil Mayers <p.mayers at imperial.ac.uk> wrote:
> Having finally got round to serious porting to 3.x, we're seeing corruption of this file on 3.0.15 and I am suspicious of the exfile locking functions which I see are now being used for this. Specifically:
> 
> https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/main/exfile.c#L332
> 
> ...seems to perform an fstat/re-open-if-unlinked check, but this happens *after* any fcntl locking has been done, and there's no attempt to re-lock the file on this re-open. So, if this codepath gets triggered, the file is now held and written to without locking, when it should be locked. Bug?

  Possibly.  I'll take a look.

> More generally, I've re-read the exfile code a couple of times and I am suspicious of the basic construction. What performance problem is the open/dup caching setup intended to solve? Put another way, why not open/close the file using the traditional posix APIs on each run?

  Because the traditional POSIX APIs are utter shit.

  If a process opens a file multiple times, and does fcntl() lock on one file descriptor, closing any OTHER file descriptor on that file causes the lock to go away.

  The only way to deal with this correctly is to add a wrapper layer around the open / lock / close, which uses mutexes for mutual exclusion and locking.  It still has to use fcntl() locking so that *other* processes trying to open the file get locked out (or not) as needed.

  i.e. you can't rely on fcntl() locks within a process.  They are *only* for inter-process locking.

  Alan DeKok.