12186 image quit problem

> Bill
>
>
> ________________________________________
> From: [hidden email] [[hidden email]] On Behalf Of Igor Stasenko [[hidden email]]
> Sent: Sunday, October 10, 2010 7:56 AM
> To: [hidden email]
> Subject: Re: [Pharo-project] 12186 image quit problem
>
> OKay, now i see what the problem:
>
> <weak registry protected>
>   <finalize items>
>       <HostSystemMenusProxy>>finalize>
>            ....
>            <HostSystemMenusProxy class>>unregister>
>              <weak registry protected>
>
>
> In my WeakRegistry, a #finalize message sent while registry semaphore locked,
> so, in case if object in #finalize trying to manipulate with registry
> then it going to deadlock.
>
> Then following piece:
>
> WeakRegistry>>finalizeValues
>
> self protected: [
>        valueDictionary expiredValuesDo: [:finItem |
>                finItem finalizeValues  ].
>  ].
>
> should be rewritten as:
>
> | expired |
> expired := OrderedCollection new.
> self protected: [
>        valueDictionary expiredValuesDo: [:finItem | expired add: finItem ]
>  ].
> expired do: #finalizeValues.
>
>
> But either way, HostSystemMenusProxy should not attempt remove itself
> from registry during #finalize (and causing a deadlock),
> since its pointless.
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> Ok, I opened an issue: http://code.google.com/p/pharo/issues/detail?id=3091
>
> The patch works for me, Thank you Igor.
>
> Cheers,
> -- Pavel
>
> On Sun, Oct 10, 2010 at 6:46 PM, Igor Stasenko <[hidden email]> wrote:
> On 10 October 2010 17:40, Schwab,Wilhelm K <[hidden email]> wrote:
> > Sig,
> >
> > Have you thought about using a Mutex?  Maybe I'm missing something in life, but long ago I decided the #forMutualExclusion is *beyond* private.  Semaphores are wonderful for #wait/#signal.  But for #critical:, I use Mutex which won't deadlock a thread with itself.
> >
>
> yes,  i thought about it. It wont help in this situation,
> because if you start modifying dictionary inside a loop, which
> scanning its elements (like #do:)
> you'll have very unpleasant results.
>
> All HostSystemMenusProxy doing during #finalize is removing itself
> from weakregistry (which is complete nonsense, since weak registry
> cleaning itself from dead stuff anyways).
> So,
>  self protected: [
>        valueDictionary expiredValuesDo: [:finItem |
>                finItem finalizeValues  ].
>  ].
>
> actually triggers a bug/mistake in HostSystemMenusProxy , which leads
> to deadlock.
> :)
>
> > Bill
> >
> >
> > ________________________________________
> > From: [hidden email] [[hidden email]] On Behalf Of Igor Stasenko [[hidden email]]
> > Sent: Sunday, October 10, 2010 7:56 AM
> > To: [hidden email]
> > Subject: Re: [Pharo-project] 12186 image quit problem
> >
> > OKay, now i see what the problem:
> >
> > <weak registry protected>
> >   <finalize items>
> >       <HostSystemMenusProxy>>finalize>
> >            ....
> >            <HostSystemMenusProxy class>>unregister>
> >              <weak registry protected>
> >
> >
> > In my WeakRegistry, a #finalize message sent while registry semaphore locked,
> > so, in case if object in #finalize trying to manipulate with registry
> > then it going to deadlock.
> >
> > Then following piece:
> >
> > WeakRegistry>>finalizeValues
> >
> > self protected: [
> >        valueDictionary expiredValuesDo: [:finItem |
> >                finItem finalizeValues  ].
> >  ].
> >
> > should be rewritten as:
> >
> > | expired |
> > expired := OrderedCollection new.
> > self protected: [
> >        valueDictionary expiredValuesDo: [:finItem | expired add: finItem ]
> >  ].
> > expired do: #finalizeValues.
> >
> >
> > But either way, HostSystemMenusProxy should not attempt remove itself
> > from registry during #finalize (and causing a deadlock),
> > since its pointless.
> >
> >
> > --
> > Best regards,
> > Igor Stasenko AKA sig.
> >
> > _______________________________________________
> > Pharo-project mailing list
> > [hidden email]
> > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
> >
> > _______________________________________________
> > Pharo-project mailing list
> > [hidden email]
> > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
> >
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

> On 11 October 2010 12:10, Stéphane Ducasse <[hidden email]> wrote:
>> I integrate
>>        Issue 3086:     MCMethodDefinition>>shutdown hack to avoid lock down.
>>        http://code.google.com/p/pharo/issues/detail?id=3086
>>
>>
>>
>> Igor can you have a look and let me know:
>>        1- if we should rollback Issue 3086:    MCMethodDefinition>>shutdown hack to avoid lock down.
>>        2- if we should integrate
>>                Issue 3048:     MC method cache fix
>>                http://code.google.com/p/pharo/issues/detail?id=3048
>>
>> I will integrate
>>        http://code.google.com/p/pharo/issues/detail?id=3091
>>
>
> Wait , MCMethodDefinition is only a consequence (however, removing it
> from weakdependents would be good idea as well ;)).

> The real cultprit is HostSystemMenusProxy , which locks the weak
> registry during finalization.
> Then MCMethodDefinition>>shutdown simply locks at same point (trying
> to obtain semaphore lock).
> Note, that HostSystemMenusProxy locks finalization process, but not
> the process which doing shutdown,
> and then during image startup, a new finalization process get created.
> That's why removing access to weakdependents in
> MCMethodDefinition>>shutdown , kinda solved the issue.
> In reality its not.
> An old weak registry were sending #finalize outside a critical section:
>
> finalizeValues
> "Some of our elements may have gone away. Look for those and activate
> the associated executors."
> | finiObjects |
> finiObjects := nil.
> "First collect the objects."
> self protected:[
> valueDictionary allAssociationsDo:[:assoc|
> assoc key isNil ifTrue:[
> finiObjects isNil
> ifTrue:[finiObjects := OrderedCollection with: assoc value]
> ifFalse:[finiObjects add: assoc value]]
> ].
> finiObjects isNil ifFalse:[valueDictionary finalizeValues:
> finiObjects asArray].
> ].
> "Then do the finalization"
> finiObjects isNil ifTrue:[^self].
> finiObjects do:[:each| each finalize].
>
>
> So, there two ways to solve it:
> a) perform finalization outside a critical section (see attached)
> b) fix the HostSystemMenusProxy to prevent any manupulation with weak
> registry during finalization.
>
>
> I checked Squeak 4.2 image, and i can also say, that any attempt to
> manipulate weak registry during finalization will also lock down a
> finalization process (see Squeak's WeakRegistry>>finalizeValues &
> WeakKeyDictionary>>finalizeValues)
>
> So, if we going to assume, in future that it is error to modify weak
> registry during finalization, then we should leave the code in
> WeakRegistry as-is, and fix HostSystemMenusProxy instead.
> If not, the we should make sure that #finalize should be sent outside
> weak registry's critical section.
>
> And the last note: if VM supports new finalization, it will happen
> outside a critical section (See the
> bottom of WeakRegistry>>finalizeValues). So any manipulations with
> registry during finalization can't cause lockdown.
>
>
> P.S. forwarded to Squeak-dev, to make Squeakers aware of potential
> danger , when manipulating weak registry during finalization.
>
> --
> Best regards,
> Igor Stasenko AKA sig.
> <WeakRegistry-finalizeValues.st>_______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

> Thanks.
> So let's see what is the best choice and tell us.
>
>
> On Oct 11, 2010, at 11:47 AM, Igor Stasenko wrote:
>
>> On 11 October 2010 12:10, Stéphane Ducasse <[hidden email]> wrote:
>>> I integrate
>>>        Issue 3086:     MCMethodDefinition>>shutdown hack to avoid lock down.
>>>        http://code.google.com/p/pharo/issues/detail?id=3086
>>>
>>>
>>>
>>> Igor can you have a look and let me know:
>>>        1- if we should rollback Issue 3086:    MCMethodDefinition>>shutdown hack to avoid lock down.
>>>        2- if we should integrate
>>>                Issue 3048:     MC method cache fix
>>>                http://code.google.com/p/pharo/issues/detail?id=3048
>>>
>>> I will integrate
>>>        http://code.google.com/p/pharo/issues/detail?id=3091
>>>
>>
>> Wait , MCMethodDefinition is only a consequence (however, removing it
>> from weakdependents would be good idea as well ;)).
>
> Yes I integrate it to avoid system hanging.
>
>
>> The real cultprit is HostSystemMenusProxy , which locks the weak
>> registry during finalization.
>> Then MCMethodDefinition>>shutdown simply locks at same point (trying
>> to obtain semaphore lock).
>> Note, that HostSystemMenusProxy locks finalization process, but not
>> the process which doing shutdown,
>> and then during image startup, a new finalization process get created.
>> That's why removing access to weakdependents in
>> MCMethodDefinition>>shutdown , kinda solved the issue.
>> In reality its not.
>> An old weak registry were sending #finalize outside a critical section:
>>
>> finalizeValues
>>       "Some of our elements may have gone away. Look for those and activate
>> the associated executors."
>>       | finiObjects |
>>       finiObjects := nil.
>>       "First collect the objects."
>>       self protected:[
>>               valueDictionary allAssociationsDo:[:assoc|
>>                       assoc key isNil ifTrue:[
>>                               finiObjects isNil
>>                                       ifTrue:[finiObjects := OrderedCollection with: assoc value]
>>                                       ifFalse:[finiObjects add: assoc value]]
>>               ].
>>               finiObjects isNil ifFalse:[valueDictionary finalizeValues:
>> finiObjects asArray].
>>       ].
>>       "Then do the finalization"
>>       finiObjects isNil ifTrue:[^self].
>>       finiObjects do:[:each| each finalize].
>>
>>
>> So, there two ways to solve it:
>> a) perform finalization outside a critical section (see attached)
>> b) fix the HostSystemMenusProxy to prevent any manupulation with weak
>> registry during finalization.
>>
>>
>> I checked Squeak 4.2 image, and i can also say, that any attempt to
>> manipulate weak registry during finalization will also lock down a
>> finalization process (see Squeak's WeakRegistry>>finalizeValues &
>> WeakKeyDictionary>>finalizeValues)
>>
>> So, if we going to assume, in future that it is error to modify weak
>> registry during finalization, then we should leave the code in
>> WeakRegistry as-is, and fix HostSystemMenusProxy instead.
>> If not, the we should make sure that #finalize should be sent outside
>> weak registry's critical section.
>>
>> And the last note: if VM supports new finalization, it will happen
>> outside a critical section (See the
>> bottom of WeakRegistry>>finalizeValues). So any manipulations with
>> registry during finalization can't cause lockdown.
>>
>>
>> P.S. forwarded to Squeak-dev, to make Squeakers aware of potential
>> danger , when manipulating weak registry during finalization.
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>> <WeakRegistry-finalizeValues.st>_______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
>