joyent/libuv

record-level locking... fs.readSafe/writeSafe

tracker1 opened this issue · 8 comments

Would be nice to have a readSafe/writeSafe methods on the filesystem object that will do range/record locking against the file being used. Since most of the OSes in place support this, it would be a really nice thing to have.

It looks like a shared lock is being done on with the open anyway, why not add the range locking for read/write access, or an additional set of methods?

@bnoordhuis did I dream about you mentioning this was not possible in a portable way?

Yes, it's pretty much impossible to implement on all supported platforms. Even on platforms where we could implement it, it won't necessarily work with NFS.

Thanks for clarifying, Ben!

@bnoordhuis but, shouldn't it be available for those platforms that can support it? It's a pretty important feature for being able to interact with data that other processes (that do range locking) are interacting with.

I understand that NFS, and even some older SAMBA/CIFS implementations may not be implemented correctly, just the same... would the underlying lock attempt throw an error if it wasn't supported? To me, that should be enough.

There are implementations for named-pipes (variances for unix and windows environments) that don't work everywhere... I would think that file/record locking would be equally important.

@tracker1 In general, libuv only implements lowest common denominator (LCD) behavior; that is, the subset that we can comfortably support on all platforms. File locking has so many gotchas and incompatibilities that there is no useful LCD to speak of.

I understand that NFS, and even some older SAMBA/CIFS implementations may not be implemented correctly, just the same... would the underlying lock attempt throw an error if it wasn't supported?

No, not necessarily. With NFS file locking:

  • The operation may be unimplemented but the operating system may still report success. (Yes, this actually happens.)
  • The operation may be implemented but buggy. (This used to be quite common.)
  • Other clients may not support the operation, making it effectively a no-op (which the operating system won't tell you about because it doesn't know that.)

Fair enough.. even if it didn't raise an error, wouldn't it be better to have the option (where the environment supports it) which would include windows and linux against local files, as opposed to not even having the option? I know it's buggy, especially with network shares, etc.. I just feel it's a really important functionality that is missing, even if support was relatively spotty.. (a warning in the docs for libuv and node.js should be enough) ...

I'm not sure what libraries, such as sqlite use for their locking... I'd presume it wasn't locking the entire file... or how their approach is for network files... I would think that matching their system would be an appropriate implementation.

I don't know if it would be worthwhile to simply apply record locking as part of the existing read/write methods... which might be better for something that may not work, depending on the underlying system... since the action may not occur and fail silently anyhow.

I don't mean to badger on about this, it's one of the few missing pieces imho.

I think sqlite uses or used to use POSIX file locks with a separate lock file. You are probably aware of this but, for posterity, this document describes what is wrong with POSIX file locks: they are process-wide and automatically dropped when any file descriptor referring to the locked file is closed. It's pretty much impossible to use them safely in a library.

Linux added file-private locks in v3.15 (afterwards renamed to open file description locks) that alleviate much of the pain with POSIX locks but that's a too recent addition to be useful. As a data point, most of StrongLoop's customers will be running 2.6.18 until at least 2017...

I know it's wrapped in another project, but Synchronet uses the following (XPDEV) as a cross-platform wrapper, and you can see how it does the file locking... it may be worth at least considering... again, it just feels like a feature that should be there, and would be very useful...

http://cvs.synchro.net/cgi-bin/viewcvs.cgi/src/xpdev/filewrap.c?view=markup Line 90 starts lock then unlock.