[PATCH] D80900: [clangd] Use different FS in PreambleThread

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov.
Herald added a project: clang.

As vfs::FileSystem is not threadsafe, we've introduced a data race when
PreambleThread started to run in async mode. As both ASTWorker and
PreambleWorker can concurrently work on the same FS.

This patch fixes that data race by propagating FSProvider into TUScheduler and
ASTWorker and making use of a new FS when issuing update to the PreambleThread.

This doesn't break our contract on FSProvider::getFileSystem, as it says the
active context will be client's context at the time of request and
ASTWorker::update is run with the context we've captured on
ClangdServer::AddDocument.

As for the operations, this implies preamble and ast threads can see different
versions of the filesystems. For example in presence of a snapshotted filesystem

- client makes an AddDocument request with FS at snapshot v1
- client then updates the FS to v2
- ASTWorker starts handling the update, caches the FS v1 in FileInputs, which is used for serving reads.
- Schedules a preamble update, this fetches a new FS from provider, which might yield v2. This says `might` as clients can handle this case by storing snapshot version in context.
- Preamble build finishes and clangd publishes diagnostics for FS v2.
- Client issues a read (e.g. go-to-def), clangd uses FS v1 for serving the request.

This can be overcome by 2 filesystems into TUScheduler in
ClangdServer::AddDocument, but I don't think it is an issue in practice
currently. As our beloved snapshotted filesystem provider stores the snapshot
version in context, and should always return the same FS for requests coming
from the same context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80900

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80900.267528.patch
Type: text/x-patch
Size: 17269 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200531/2cd78b16/attachment-0001.bin>