[lld] r264022 - Use owning pointers instead of raw pointers for Atom's to fix leaks.

[David Blaikie via llvm-commits]

On Mon, Mar 21, 2016 at 9:08 PM, Pete Cooper <peter_cooper at apple.com> wrote:

>
> On Mar 21, 2016, at 9:05 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Mon, Mar 21, 2016 at 8:44 PM, Pete Cooper via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: pete
>> Date: Mon Mar 21 22:44:32 2016
>> New Revision: 264022
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=264022&view=rev
>> Log:
>> Use owning pointers instead of raw pointers for Atom's to fix leaks.
>>
>> Currently each File contains an BumpPtrAllocator in which Atom's are
>> allocated.  Some Atom's contain data structures like std::vector which
>> leak as we don't run ~Atom when they are BumpPtrAllocate'd.
>>
>
> FWIW, if the only thing allocated in the BumpPtrAllocator is Atoms, you
> could use a SpecificBumpPtrAllocator, which does run the doors.
>
> I think there might be Reference’s in there, but worth investigating doing
> it this way.  Unfortunately the Window’s bots didn’t like what I did here
> so i’ve reverted for now.
>
>
> (& if the BumpPtrAllocator doesn't contain only Atoms, you could change it
> so it does)
>
>
>>
>> Now each File actually owns its Atom's using an OwningAtomPtr.  This
>> is analygous to std::unique_ptr and may be replaced by it if possible.
>>
>
> Yeah, this looks like it could just be a typedef of unique_ptr with a
> custom deleter that only runs the dtor but doesn't delete, etc.
>
> Just tried this as a quick fix to get the bots to work.  Unfortunately the
> YAML parser doesn’t like that solution. The problem is that the YAML parser
> wants to take a reference to a pointer, so i had to make the get() methods
> be defined as follows to support that.  std::unique_ptr doesn’t return a
> reference from get() so the YAML parser stops working with them.
>

Ah, yeah, that's not a safe operation (in the mutable case, especially) for
a smart pointer - so if we're going to smart pointerify these things,
probably need to adjust that code no matter which smart pointer we use.

Did you try making the dtor virtual to fix MSVC's issues? If you are
destroying derived objects your dtor does need to be virtual if you're
calling it from a base object expression.

>
>
>>
>> +  T *const &get() const {
>> +    return atom;
>> +  }
>> +
>> +  T *&get() {
>> +    return atom;
>> +  }
>> +
>
>
>>
> Cheers,
> Pete
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160321/182296f1/attachment.html>