Author: kadircet Date: Wed Oct 9 06:59:31 2019 New Revision: 374168 URL: http://llvm.org/viewvc/llvm-project?rev=374168&view=rev Log: [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 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=374168&r1=374167&r2=374168&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Oct 9 06:59:31 2019 @@ -371,16 +371,6 @@ private: llvm::StringMap<std::function<void(llvm::json::Value)>> Notifications; llvm::StringMap<std::function<void(llvm::json::Value, ReplyOnce)>> Calls; - // The maximum number of callbacks held in clangd. - // - // We bound the maximum size to the pending map to prevent memory leakage - // for cases where LSP clients don't reply for the request. - static constexpr int MaxReplayCallbacks = 100; - mutable std::mutex CallMutex; - int NextCallID = 0; /* GUARDED_BY(CallMutex) */ - std::deque<std::pair</*RequestID*/ int, - /*ReplyHandler*/ Callback<llvm::json::Value>>> - ReplyCallbacks; /* GUARDED_BY(CallMutex) */ // Method calls may be cancelled by ID, so keep track of their state. // This needs a mutex: handlers may finish on a different thread, and that's @@ -432,6 +422,19 @@ private: })); } + // The maximum number of callbacks held in clangd. + // + // We bound the maximum size to the pending map to prevent memory leakage + // for cases where LSP clients don't reply for the request. + // This has to go after RequestCancellers and RequestCancellersMutex since it + // can contain a callback that has a cancelable context. + static constexpr int MaxReplayCallbacks = 100; + mutable std::mutex CallMutex; + int NextCallID = 0; /* GUARDED_BY(CallMutex) */ + std::deque<std::pair</*RequestID*/ int, + /*ReplyHandler*/ Callback<llvm::json::Value>>> + ReplyCallbacks; /* GUARDED_BY(CallMutex) */ + ClangdLSPServer &Server; }; constexpr int ClangdLSPServer::MessageHandler::MaxReplayCallbacks; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
