llvm dev - Feb 2013 - [LLVMdev] ManagedStatic and order of destruction

I'm curious about the design rationale for how ManagedStatic instances are
cleaned up, and I'm hoping someone can shed some light on it.

Currently, ManagedStatic objects are cleaned up when llvm_shutdown()
traverses the global list of initialized objects and calls destroy() on
each.  This leads to two questions:

1. An assertion enforces that the objects are deleted in reverse order of
allocation.  Is there any hard requirement for this, or is it only because
of the way the linked list is created?

2. Would anything change (functionally) if we simply called destroy() in
the ManagedStatic destructor?  Wouldn't this be safer than relying on a
client to call llvm_shutdown()?  It seems like the whole purpose of
llvm_shutdown() is to allow clients to free the data allocated by
ManagedStatic instances.  Could we not just give them an option, but by
default clean up during global destruction?

-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130209/e9354c2e/attachment.html>

Ping.  Anyone with knowledge of detailed knowledge of the ManagedStatic
implementation?

The use-case we are looking at is getting rid of llvm_shutdown() by
cleaning up the ManagedStatic data when the global destructors run.


On Sat, Feb 9, 2013 at 5:25 PM, Justin Holewinski <
justin.holewinski at gmail.com> wrote:
> I'm curious about the design rationale for how ManagedStatic instances
are
> cleaned up, and I'm hoping someone can shed some light on it.
>
> Currently, ManagedStatic objects are cleaned up when llvm_shutdown()
> traverses the global list of initialized objects and calls destroy() on
> each.  This leads to two questions:
>
> 1. An assertion enforces that the objects are deleted in reverse order of
> allocation.  Is there any hard requirement for this, or is it only because
> of the way the linked list is created?
>
> 2. Would anything change (functionally) if we simply called destroy() in
> the ManagedStatic destructor?  Wouldn't this be safer than relying on a
> client to call llvm_shutdown()?  It seems like the whole purpose of
> llvm_shutdown() is to allow clients to free the data allocated by
> ManagedStatic instances.  Could we not just give them an option, but by
> default clean up during global destruction?
>
> --
>
> Thanks,
>
> Justin Holewinski
>


-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130212/620ce9f1/attachment.html>

Right, I'm suggesting we keep llvm_shutdown() for users who want this
control, but also destroy still-live ManagedStatic instances if
llvm_shutdown() is not called.  This helps in the case where there is not a
clear time when llvm_shutdown() can be called, especially given that LLVM
cannot be resurrected in the same process due to current limitations in the
pass registry, and perhaps elsewhere.


On Wed, Feb 13, 2013 at 7:47 AM, Nicholas Chapman
<admin at indigorenderer.com>wrote:
>  Hi Justin,
> I don't know anything about the details of llvm_shutdown and the
> ManagedStatic implementation.
> But it's nice to be able to explicitly free memory allocated by LLVM,
> otherwise such memory (allocated by global objects) can show up as memory
> leaks.
>
> Thanks,
>     Nick C.
>
>  On 12/02/2013 6:31 p.m., Justin Holewinski wrote:
>
> Ping.  Anyone with knowledge of detailed knowledge of the ManagedStatic
> implementation?
>
>  The use-case we are looking at is getting rid of llvm_shutdown() by
> cleaning up the ManagedStatic data when the global destructors run.
>
>
> On Sat, Feb 9, 2013 at 5:25 PM, Justin Holewinski <
> justin.holewinski at gmail.com> wrote:
>
>> I'm curious about the design rationale for how ManagedStatic
instances
>> are cleaned up, and I'm hoping someone can shed some light on it.
>>
>>  Currently, ManagedStatic objects are cleaned up when llvm_shutdown()
>> traverses the global list of initialized objects and calls destroy() on
>> each.  This leads to two questions:
>>
>>  1. An assertion enforces that the objects are deleted in reverse order
>> of allocation.  Is there any hard requirement for this, or is it only
>> because of the way the linked list is created?
>>
>>  2. Would anything change (functionally) if we simply called destroy()
>> in the ManagedStatic destructor?  Wouldn't this be safer than
relying on a
>> client to call llvm_shutdown()?  It seems like the whole purpose of
>> llvm_shutdown() is to allow clients to free the data allocated by
>> ManagedStatic instances.  Could we not just give them an option, but by
>> default clean up during global destruction?
>>
>>  --
>>
>> Thanks,
>>
>>  Justin Holewinski
>>
>
>
>
>  --
>
> Thanks,
>
>  Justin Holewinski
>
>
> _______________________________________________
> LLVM Developers mailing listLLVMdev at cs.uiuc.edu        
http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>

-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130213/f3e362c3/attachment.html>

Looking at this some more, there appears to be some cases where LLVM is
causing memory leaks due to ManagedStatic (and perhaps undefined behavior).


Example:

The expected client behavior is to call llvm_shutdown() before returning
from main().  This causes destroy() to be called on all ManagedStatic
instances that have been accessed so far, deleting the child data and
NULLing out the StaticList global.  This includes PassRegistry.

After main terminates, the global destructors run, including the
command-line option destructors.  The "-print-before" and
"-print-after"
options use a PassNameParser instance.  This parser is a sub-class of
PassRegistrationListener, and the destructor for this class calls
PassRegistry::getPassRegistry().  This call accesses a
ManagedStatic<PassRegistry> instance.  But, all ManagedStatic instances
have already been cleaned up!  This brings up two issues/questions:

1. The ManagedStatic instance was cleaned-up, so the accessors re-create
the object.  Since all ManagedStatic instances were already cleaned-up,
this new instance is leaked.

2. Is this not undefined behavior?  There is no guarantee which objects are
destructed when, so is it even valid to assume that the ManagedStatic
instance is still usable?  Couldn't the compile decide to write all 1s to
the memory range used by the ManagedStatic instance?  Or is there some
guarantee that ManagedStatic is a POD type and won't be touched until after
all global destructors run?



On Wed, Feb 13, 2013 at 7:54 AM, Justin Holewinski <
justin.holewinski at gmail.com> wrote:
> Right, I'm suggesting we keep llvm_shutdown() for users who want this
> control, but also destroy still-live ManagedStatic instances if
> llvm_shutdown() is not called.  This helps in the case where there is not a
> clear time when llvm_shutdown() can be called, especially given that LLVM
> cannot be resurrected in the same process due to current limitations in the
> pass registry, and perhaps elsewhere.
>
>
> On Wed, Feb 13, 2013 at 7:47 AM, Nicholas Chapman <
> admin at indigorenderer.com> wrote:
>
>>  Hi Justin,
>> I don't know anything about the details of llvm_shutdown and the
>> ManagedStatic implementation.
>> But it's nice to be able to explicitly free memory allocated by
LLVM,
>> otherwise such memory (allocated by global objects) can show up as
memory
>> leaks.
>>
>> Thanks,
>>     Nick C.
>>
>>  On 12/02/2013 6:31 p.m., Justin Holewinski wrote:
>>
>> Ping.  Anyone with knowledge of detailed knowledge of the ManagedStatic
>> implementation?
>>
>>  The use-case we are looking at is getting rid of llvm_shutdown() by
>> cleaning up the ManagedStatic data when the global destructors run.
>>
>>
>> On Sat, Feb 9, 2013 at 5:25 PM, Justin Holewinski <
>> justin.holewinski at gmail.com> wrote:
>>
>>> I'm curious about the design rationale for how ManagedStatic
instances
>>> are cleaned up, and I'm hoping someone can shed some light on
it.
>>>
>>>  Currently, ManagedStatic objects are cleaned up when
llvm_shutdown()
>>> traverses the global list of initialized objects and calls
destroy() on
>>> each.  This leads to two questions:
>>>
>>>  1. An assertion enforces that the objects are deleted in reverse
order
>>> of allocation.  Is there any hard requirement for this, or is it
only
>>> because of the way the linked list is created?
>>>
>>>  2. Would anything change (functionally) if we simply called
destroy()
>>> in the ManagedStatic destructor?  Wouldn't this be safer than
relying on a
>>> client to call llvm_shutdown()?  It seems like the whole purpose of
>>> llvm_shutdown() is to allow clients to free the data allocated by
>>> ManagedStatic instances.  Could we not just give them an option,
but by
>>> default clean up during global destruction?
>>>
>>>  --
>>>
>>> Thanks,
>>>
>>>  Justin Holewinski
>>>
>>
>>
>>
>>  --
>>
>> Thanks,
>>
>>  Justin Holewinski
>>
>>
>> _______________________________________________
>> LLVM Developers mailing listLLVMdev at cs.uiuc.edu        
http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>>
>
>
> --
>
> Thanks,
>
> Justin Holewinski
>


-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130219/c60c5215/attachment.html>