Skip to content

Commit 9a3a87d

Browse files
committed
[clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex
Summary: After rL374163, replycallbacks might have a cancellable context, which will try to access RequestCancellers on destruction. See http://45.33.8.238/mac/1245/step_7.txt for a sample failure. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits, thakis Tags: #clang Differential Revision: https://reviews.llvm.org/D68702 llvm-svn: 374168
1 parent 2ae54ab commit 9a3a87d

File tree

1 file changed

+13
-10
lines changed

1 file changed

+13
-10
lines changed

clang-tools-extra/clangd/ClangdLSPServer.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -371,16 +371,6 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
371371

372372
llvm::StringMap<std::function<void(llvm::json::Value)>> Notifications;
373373
llvm::StringMap<std::function<void(llvm::json::Value, ReplyOnce)>> Calls;
374-
// The maximum number of callbacks held in clangd.
375-
//
376-
// We bound the maximum size to the pending map to prevent memory leakage
377-
// for cases where LSP clients don't reply for the request.
378-
static constexpr int MaxReplayCallbacks = 100;
379-
mutable std::mutex CallMutex;
380-
int NextCallID = 0; /* GUARDED_BY(CallMutex) */
381-
std::deque<std::pair</*RequestID*/ int,
382-
/*ReplyHandler*/ Callback<llvm::json::Value>>>
383-
ReplyCallbacks; /* GUARDED_BY(CallMutex) */
384374

385375
// Method calls may be cancelled by ID, so keep track of their state.
386376
// This needs a mutex: handlers may finish on a different thread, and that's
@@ -432,6 +422,19 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
432422
}));
433423
}
434424

425+
// The maximum number of callbacks held in clangd.
426+
//
427+
// We bound the maximum size to the pending map to prevent memory leakage
428+
// for cases where LSP clients don't reply for the request.
429+
// This has to go after RequestCancellers and RequestCancellersMutex since it
430+
// can contain a callback that has a cancelable context.
431+
static constexpr int MaxReplayCallbacks = 100;
432+
mutable std::mutex CallMutex;
433+
int NextCallID = 0; /* GUARDED_BY(CallMutex) */
434+
std::deque<std::pair</*RequestID*/ int,
435+
/*ReplyHandler*/ Callback<llvm::json::Value>>>
436+
ReplyCallbacks; /* GUARDED_BY(CallMutex) */
437+
435438
ClangdLSPServer &Server;
436439
};
437440
constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks;

0 commit comments

Comments
 (0)