[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D54995#1314912 <https://reviews.llvm.org/D54995#1314912>, @zturner wrote:

> Original patch description says this:
>
> > There are reported cases of non-system files being locked by libclang on Windows (and likely by other clients as well)
>
> What is the nature of the reports?  What operation is attempted on the files and fails due to locking?  And which application is it that's failing and in what way?


https://bugreports.qt.io/browse/QTCREATORBUG-15449 was mentioned previously in the thread.

I've tried reproducing with the provided example and succeeded: if I map a view of the file, none of the editors would be able to write into that file.
I tried various combinations of flags, trying really hard to do readonly and copy-on-write mapping, but none would work, a file cannot be written into while there are active mapped views of the it.

E.g. for VSCode process monitor reports it eventually tries to call SetEndOfFile <https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-setendoffile> and fails with `ERROR_USER_MAPPED_FILE` error (it can be found in the list of errors <https://docs.microsoft.com/en-gb/windows/desktop/Debug/system-error-codes--1000-1299-> from Microsoft).
There's this also Stackoverflow question <https://stackoverflow.com/questions/41844842/when-error-1224-error-user-mapped-file-occurs> mentioning some cases where this occurs.

It looks like there's no workaround for this case in the Windows APIs, so I guess we'll have to trade-off memory in that case and read the whole file using the classical file APIs.

@yvvan, I was proven wrong. Thanks for taking your time to go through the trouble of providing this example. I'm not sure why my previous experiments came up with a different result, I've probably made a mistake somewhere along the way.
Nevertheless, could we avoid changing the defaults to `true`? We probably forgot to propagate the flag somewhere, those places should be easy to find by **temporarily** removing the default args, rerunning the build for libclang and then bringing the default args back.
It's fine to enable this behavior for editors, but various command-line tools, including the compiler, are definitely better off using the memory-mapped files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54995/new/

https://reviews.llvm.org/D54995