[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well... like they say, there's no software engineering problem that can't be solved by adding an extra layer of indirection.

In this case, that's "adding an extra layer and leaking it", so I'm wondering if we can drop the second part. I'm going to assume that we want to preserve `sizeof(ptr<TypeSystem>)`, even though I'm not entirely convinced that this would cause a significant increase in our memory footprint.

An ideal solution for this would be a `IntrusiveRefCntPtr`, but with weak pointer support. That doesn't exist right now, but since we're already adding a layer of indirection, I think that layer could be used to introduce weak semantics. Instead of this "canonical" `TypeSystemWP *`, we could pass around a `IntrusiveRefCntPtr<TypeSystemHolder>`, where `TypeSystemHolder` is essentially a `TypeSystemWP` wrapped in a `ThreadSafeRefCountedBase`. I think this doesn't increase the number of dereferences (it's still takes two derefs to get from this funky representation to an actual type system), but it has the advantage that the type system holder will go away when/if all references to it are gone. Another advantage (I hope) could be that we don't see this funky double deref (`TypeSystemWP *`) pattern everywhere, as it's helpfully hidden in the holder class.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136650/new/

https://reviews.llvm.org/D136650