[PATCH] D92480: [llvm] Add asserts in (ThreadSafe)?RefCountedBase destructors
Nathan James via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 4 02:22:19 PST 2020
njames93 added inline comments.
================
Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:190-196
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+ static void call(void *Ptr) {
+ static_cast<DynMatcherInterface *>(Ptr)->Release();
+ }
+};
----------------
dblaikie wrote:
> I think probably the right solution is to have TrueMatcherImpl's dtor Release the same way its ctor Retains. Symmetry is nice/helps avoid splitting this quirk into two different places.
That method is a nasty can of worms. If TrueMatcherImpl dtor calls `Release`, that `Release` call would realise the RefCount is now zero. That in turn would then call delete on `TrueMatcherImpl` resulting in a double-free.
I think symmetry is possibly the right way to go though, just in using a custom creator that calls Retain itself, remove that responsibility from TrueMatcherImpl.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92480/new/
https://reviews.llvm.org/D92480
More information about the llvm-commits
mailing list