Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -233,6 +233,13 @@ std::string dumpAST(PathRef File); private: + std::future + scheduleReparseAndDiags(PathRef File, VersionedDraft Contents, + std::shared_ptr Resources, + Tagged> TaggedFS); + + std::future scheduleCancelRebuild(std::shared_ptr Resources); + GlobalCompilationDatabase &CDB; DiagnosticsConsumer &DiagConsumer; FileSystemProvider &FSProvider; Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -143,61 +143,14 @@ auto TaggedFS = FSProvider.getTaggedFileSystem(File); std::shared_ptr Resources = Units.getOrCreateFile(File, ResourceDir, CDB, PCHs, TaggedFS.Value); - - std::future>> DeferredRebuild = - Resources->deferRebuild(Contents, TaggedFS.Value); - std::promise DonePromise; - std::future DoneFuture = DonePromise.get_future(); - - Path FileStr = File; - VFSTag Tag = TaggedFS.Tag; - auto ReparseAndPublishDiags = - [this, FileStr, Version, - Tag](std::future>> - DeferredRebuild, - std::promise DonePromise) -> void { - FulfillPromiseGuard Guard(DonePromise); - - auto CurrentVersion = DraftMgr.getVersion(FileStr); - if (CurrentVersion != Version) - return; // This request is outdated - - auto Diags = DeferredRebuild.get(); - if (!Diags) - return; // A new reparse was requested before this one completed. - DiagConsumer.onDiagnosticsReady(FileStr, - make_tagged(std::move(*Diags), Tag)); - }; - - WorkScheduler.addToFront(std::move(ReparseAndPublishDiags), - std::move(DeferredRebuild), std::move(DonePromise)); - return DoneFuture; + return scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()}, + std::move(Resources), std::move(TaggedFS)); } std::future ClangdServer::removeDocument(PathRef File) { - auto Version = DraftMgr.removeDraft(File); - Path FileStr = File; - - std::promise DonePromise; - std::future DoneFuture = DonePromise.get_future(); - - auto RemoveDocFromCollection = [this, FileStr, - Version](std::promise DonePromise) { - FulfillPromiseGuard Guard(DonePromise); - - if (Version != DraftMgr.getVersion(FileStr)) - return; // This request is outdated, do nothing - - std::shared_ptr File = Units.removeIfPresent(FileStr); - if (!File) - return; - // Cancel all ongoing rebuilds, so that we don't do extra work before - // deleting this file. - File->cancelRebuilds(); - }; - WorkScheduler.addToFront(std::move(RemoveDocFromCollection), - std::move(DonePromise)); - return DoneFuture; + DraftMgr.removeDraft(File); + std::shared_ptr Resources = Units.removeIfPresent(File); + return scheduleCancelRebuild(std::move(Resources)); } std::future ClangdServer::forceReparse(PathRef File) { @@ -306,3 +259,60 @@ }); return make_tagged(std::move(Result), TaggedFS.Tag); } + +std::future ClangdServer::scheduleReparseAndDiags( + PathRef File, VersionedDraft Contents, std::shared_ptr Resources, + Tagged> TaggedFS) { + + assert(Contents.Draft && "Draft must have contents"); + std::future>> DeferredRebuild = + Resources->deferRebuild(*Contents.Draft, TaggedFS.Value); + std::promise DonePromise; + std::future DoneFuture = DonePromise.get_future(); + + DocVersion Version = Contents.Version; + Path FileStr = File; + VFSTag Tag = TaggedFS.Tag; + auto ReparseAndPublishDiags = + [this, FileStr, Version, + Tag](std::future>> + DeferredRebuild, + std::promise DonePromise) -> void { + FulfillPromiseGuard Guard(DonePromise); + + auto CurrentVersion = DraftMgr.getVersion(FileStr); + if (CurrentVersion != Version) + return; // This request is outdated + + auto Diags = DeferredRebuild.get(); + if (!Diags) + return; // A new reparse was requested before this one completed. + DiagConsumer.onDiagnosticsReady(FileStr, + make_tagged(std::move(*Diags), Tag)); + }; + + WorkScheduler.addToFront(std::move(ReparseAndPublishDiags), + std::move(DeferredRebuild), std::move(DonePromise)); + return DoneFuture; +} + +std::future +ClangdServer::scheduleCancelRebuild(std::shared_ptr Resources) { + std::promise DonePromise; + std::future DoneFuture = DonePromise.get_future(); + if (!Resources) { + // No need to schedule any cleanup. + DonePromise.set_value(); + return DoneFuture; + } + + std::future DeferredCancel = Resources->deferCancelRebuild(); + auto CancelReparses = [Resources](std::promise DonePromise, + std::future DeferredCancel) { + FulfillPromiseGuard Guard(DonePromise); + DeferredCancel.get(); + }; + WorkScheduler.addToFront(std::move(CancelReparses), std::move(DonePromise), + std::move(DeferredCancel)); + return DoneFuture; +} Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -151,9 +151,17 @@ CppFile(CppFile const &) = delete; CppFile(CppFile &&) = delete; - /// Cancels all scheduled rebuilds and sets AST and Preamble to nulls. + /// Cancels a scheduled rebuild, if any, and sets AST and Preamble to nulls. /// If a rebuild is in progress, will wait for it to finish. - void cancelRebuilds(); + void cancelRebuild(); + + /// Similar to deferRebuild, but sets both Preamble and AST to nulls instead + /// of doing an actual parsing. Returned future is a deferred computation that + /// will wait for any ongoing rebuilds to finish and actually set the AST and + /// Preamble to nulls. It can be run on a different thread. + /// This function is useful to cancel ongoing rebuilds, if any, before + /// removing CppFile. + std::future deferCancelRebuild(); /// Rebuild AST and Preamble synchronously on the calling thread. /// Returns a list of diagnostics or a llvm::None, if another rebuild was Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -711,10 +711,12 @@ ASTFuture = ASTPromise.get_future(); } -void CppFile::cancelRebuilds() { +void CppFile::cancelRebuild() { deferCancelRebuild().get(); } + +std::future CppFile::deferCancelRebuild() { std::unique_lock Lock(Mutex); // Cancel an ongoing rebuild, if any, and wait for it to finish. - ++this->RebuildCounter; + unsigned RequestRebuildCounter = ++this->RebuildCounter; // Rebuild asserts that futures aren't ready if rebuild is cancelled. // We want to keep this invariant. if (futureIsReady(PreambleFuture)) { @@ -725,12 +727,28 @@ ASTPromise = std::promise>(); ASTFuture = ASTPromise.get_future(); } - // Now wait for rebuild to finish. - RebuildCond.wait(Lock, [this]() { return !this->RebuildInProgress; }); - // Return empty results for futures. - PreamblePromise.set_value(nullptr); - ASTPromise.set_value(std::make_shared(llvm::None)); + Lock.unlock(); + // Notify about changes to RebuildCounter. + RebuildCond.notify_all(); + + std::shared_ptr That = shared_from_this(); + return std::async(std::launch::deferred, [That, RequestRebuildCounter]() { + std::unique_lock Lock(That->Mutex); + CppFile *This = &*That; + This->RebuildCond.wait(Lock, [This, RequestRebuildCounter]() { + return !This->RebuildInProgress || + This->RebuildCounter != RequestRebuildCounter; + }); + + // This computation got cancelled itself, do nothing. + if (This->RebuildCounter != RequestRebuildCounter) + return; + + // Set empty results for Promises. + That->PreamblePromise.set_value(nullptr); + That->ASTPromise.set_value(std::make_shared(llvm::None)); + }); } llvm::Optional> @@ -767,6 +785,8 @@ this->ASTFuture = this->ASTPromise.get_future(); } } // unlock Mutex. + // Notify about changes to RebuildCounter. + RebuildCond.notify_all(); // A helper to function to finish the rebuild. May be run on a different // thread. @@ -916,7 +936,10 @@ if (WasCancelledBeforeConstruction) return; - File.RebuildCond.wait(Lock, [&File]() { return !File.RebuildInProgress; }); + File.RebuildCond.wait(Lock, [&File, RequestRebuildCounter]() { + return !File.RebuildInProgress || + File.RebuildCounter != RequestRebuildCounter; + }); WasCancelledBeforeConstruction = File.RebuildCounter != RequestRebuildCounter; if (WasCancelledBeforeConstruction)