broken updateStream

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

broken updateStream

Nicolas Cellier
Hi,
I tried updating a relatively recent image from:

http://files.squeak.org/6.0alpha/Squeak6.0alpha-16548-32bit/

What happens to me is an unhandled exception MNU Context>>terminateTo: during update 407.

Update 407 is loading Kernel.eem.1078 which is storing the new Kernel classes (Context rather than MethodContext)

Last change written in the change log is:

    Context removeSelector: #terminateTo:

Oh, but there's no such instruction in Kernel-eem.1078...
So what happens exactly which told the image to remove some just installed Context methods???

What happens before that is the boostrapContext in Kernel-eem.1077 preamble

    Smalltalk renameClassNamed: #MethodContext as: #Context

And what happens when we rename is in Environment>>renameClass:from:to:

    oldBinding := self declarationOf: oldName.
    declarations removeKey: oldName.
    self binding: oldBinding removedFrom: self.
    " re-route now undeclared oldBinding "
    oldBinding value: aClass.

That means that we have an Undeclared #MethodeContext => Context.

And what happens when we ask to remove MethodContext class?
We start removing Context!!!

So what might be necessary is to purgeUndeclared in Kernel-eem.1078 preamble.
And it will work if and only if we removed all MethodContext references.







Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier
Ah, but thanks to my previous WeakIdentityDictionary changes, Undeclared holds weakly to its binding.
And after update 406, the references to MethodContext have already vanished.
So maybe a simple garbageCollect might help update 407 to proceed without any problem?

I've just tried that in a package above Kernel in the configuration map....
But it does not seems to work. I'll see tomorrow.

2017-04-07 23:07 GMT+02:00 Nicolas Cellier <[hidden email]>:
Hi,
I tried updating a relatively recent image from:

http://files.squeak.org/6.0alpha/Squeak6.0alpha-16548-32bit/

What happens to me is an unhandled exception MNU Context>>terminateTo: during update 407.

Update 407 is loading Kernel.eem.1078 which is storing the new Kernel classes (Context rather than MethodContext)

Last change written in the change log is:

    Context removeSelector: #terminateTo:

Oh, but there's no such instruction in Kernel-eem.1078...
So what happens exactly which told the image to remove some just installed Context methods???

What happens before that is the boostrapContext in Kernel-eem.1077 preamble

    Smalltalk renameClassNamed: #MethodContext as: #Context

And what happens when we rename is in Environment>>renameClass:from:to:

    oldBinding := self declarationOf: oldName.
    declarations removeKey: oldName.
    self binding: oldBinding removedFrom: self.
    " re-route now undeclared oldBinding "
    oldBinding value: aClass.

That means that we have an Undeclared #MethodeContext => Context.

And what happens when we ask to remove MethodContext class?
We start removing Context!!!

So what might be necessary is to purgeUndeclared in Kernel-eem.1078 preamble.
And it will work if and only if we removed all MethodContext references.








Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier
Ah no, the binding #MethodContext=>Context is still in the Smalltalk globals declarations (and bindings) after update 406.
It's not at all in Undeclared. Why is that? Oh, no, it's the bootstrapContext method:

    Smalltalk at: #MethodContext ifAbsentPut: [Smalltalk classNamed: #Context].

And there are still plenty references to this binding at this stage.

So why loading Kernel-eem.1078 manually doesn't do any arm, while loading update-eem.407.mcm does?
This time it'll be for tomorrow

2017-04-08 0:21 GMT+02:00 Nicolas Cellier <[hidden email]>:
Ah, but thanks to my previous WeakIdentityDictionary changes, Undeclared holds weakly to its binding.
And after update 406, the references to MethodContext have already vanished.
So maybe a simple garbageCollect might help update 407 to proceed without any problem?

I've just tried that in a package above Kernel in the configuration map....
But it does not seems to work. I'll see tomorrow.

2017-04-07 23:07 GMT+02:00 Nicolas Cellier <[hidden email]>:
Hi,
I tried updating a relatively recent image from:

http://files.squeak.org/6.0alpha/Squeak6.0alpha-16548-32bit/

What happens to me is an unhandled exception MNU Context>>terminateTo: during update 407.

Update 407 is loading Kernel.eem.1078 which is storing the new Kernel classes (Context rather than MethodContext)

Last change written in the change log is:

    Context removeSelector: #terminateTo:

Oh, but there's no such instruction in Kernel-eem.1078...
So what happens exactly which told the image to remove some just installed Context methods???

What happens before that is the boostrapContext in Kernel-eem.1077 preamble

    Smalltalk renameClassNamed: #MethodContext as: #Context

And what happens when we rename is in Environment>>renameClass:from:to:

    oldBinding := self declarationOf: oldName.
    declarations removeKey: oldName.
    self binding: oldBinding removedFrom: self.
    " re-route now undeclared oldBinding "
    oldBinding value: aClass.

That means that we have an Undeclared #MethodeContext => Context.

And what happens when we ask to remove MethodContext class?
We start removing Context!!!

So what might be necessary is to purgeUndeclared in Kernel-eem.1078 preamble.
And it will work if and only if we removed all MethodContext references.









Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

David T. Lewis
To follow up on another idea, I tried reorganizing MCMcmUpdater such that
#doUpdate: processes one MCM at a time, restarting itself after each MCM
by calling itself with a Future.


        config = #restart ifTrue: [
                ^self future doUpdate: interactive
        ].


Sorry to say, it does not work. The update hangs up at the same place
with the same error. So this seems to be not a solution.

Dave


On Sat, Apr 08, 2017 at 12:45:01AM +0200, Nicolas Cellier wrote:

> Ah no, the binding #MethodContext=>Context is still in the Smalltalk
> globals declarations (and bindings) after update 406.
> It's not at all in Undeclared. Why is that? Oh, no, it's the
> bootstrapContext method:
>
>     Smalltalk at: #MethodContext ifAbsentPut: [Smalltalk classNamed:
> #Context].
>
> And there are still plenty references to this binding at this stage.
>
> So why loading Kernel-eem.1078 manually doesn't do any arm, while loading
> update-eem.407.mcm does?
> This time it'll be for tomorrow
>
> 2017-04-08 0:21 GMT+02:00 Nicolas Cellier <
> [hidden email]>:
>
> > Ah, but thanks to my previous WeakIdentityDictionary changes, Undeclared
> > holds weakly to its binding.
> > And after update 406, the references to MethodContext have already
> > vanished.
> > So maybe a simple garbageCollect might help update 407 to proceed without
> > any problem?
> >
> > I've just tried that in a package above Kernel in the configuration map....
> > But it does not seems to work. I'll see tomorrow.
> >
> > 2017-04-07 23:07 GMT+02:00 Nicolas Cellier <nicolas.cellier.aka.nice@
> > gmail.com>:
> >
> >> Hi,
> >> I tried updating a relatively recent image from:
> >>
> >> http://files.squeak.org/6.0alpha/Squeak6.0alpha-16548-32bit/
> >>
> >> What happens to me is an unhandled exception MNU Context>>terminateTo:
> >> during update 407.
> >>
> >> Update 407 is loading Kernel.eem.1078 which is storing the new Kernel
> >> classes (Context rather than MethodContext)
> >>
> >> Last change written in the change log is:
> >>
> >>     Context removeSelector: #terminateTo:
> >>
> >> Oh, but there's no such instruction in Kernel-eem.1078...
> >> So what happens exactly which told the image to remove some just
> >> installed Context methods???
> >>
> >> What happens before that is the boostrapContext in Kernel-eem.1077
> >> preamble
> >>
> >>     Smalltalk renameClassNamed: #MethodContext as: #Context
> >>
> >> And what happens when we rename is in Environment>>renameClass:from:to:
> >>
> >>     oldBinding := self declarationOf: oldName.
> >>     declarations removeKey: oldName.
> >>     self binding: oldBinding removedFrom: self.
> >>     " re-route now undeclared oldBinding "
> >>     oldBinding value: aClass.
> >>
> >> That means that we have an Undeclared #MethodeContext => Context.
> >>
> >> And what happens when we ask to remove MethodContext class?
> >> We start removing Context!!!
> >>
> >> So what might be necessary is to purgeUndeclared in Kernel-eem.1078
> >> preamble.
> >> And it will work if and only if we removed all MethodContext references.
> >>
> >>
> >>
> >>
> >>
> >>
> >

>


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier
In reply to this post by Nicolas Cellier
My finding today is that ContextPart still have a subclass named Context after update 406.

ContextPart allSubclasses collect: #superclass.
-> an OrderedCollection(ContextPart InstructionStream)

So I will put a conditional breakpoint in removeSelector: to identify the path that remove the selector and re-launch the update

2017-04-08 0:45 GMT+02:00 Nicolas Cellier <[hidden email]>:
Ah no, the binding #MethodContext=>Context is still in the Smalltalk globals declarations (and bindings) after update 406.
It's not at all in Undeclared. Why is that? Oh, no, it's the bootstrapContext method:

    Smalltalk at: #MethodContext ifAbsentPut: [Smalltalk classNamed: #Context].

And there are still plenty references to this binding at this stage.

So why loading Kernel-eem.1078 manually doesn't do any arm, while loading update-eem.407.mcm does?
This time it'll be for tomorrow


2017-04-08 0:21 GMT+02:00 Nicolas Cellier <[hidden email]>:
Ah, but thanks to my previous WeakIdentityDictionary changes, Undeclared holds weakly to its binding.
And after update 406, the references to MethodContext have already vanished.
So maybe a simple garbageCollect might help update 407 to proceed without any problem?

I've just tried that in a package above Kernel in the configuration map....
But it does not seems to work. I'll see tomorrow.

2017-04-07 23:07 GMT+02:00 Nicolas Cellier <[hidden email]>:
Hi,
I tried updating a relatively recent image from:

http://files.squeak.org/6.0alpha/Squeak6.0alpha-16548-32bit/

What happens to me is an unhandled exception MNU Context>>terminateTo: during update 407.

Update 407 is loading Kernel.eem.1078 which is storing the new Kernel classes (Context rather than MethodContext)

Last change written in the change log is:

    Context removeSelector: #terminateTo:

Oh, but there's no such instruction in Kernel-eem.1078...
So what happens exactly which told the image to remove some just installed Context methods???

What happens before that is the boostrapContext in Kernel-eem.1077 preamble

    Smalltalk renameClassNamed: #MethodContext as: #Context

And what happens when we rename is in Environment>>renameClass:from:to:

    oldBinding := self declarationOf: oldName.
    declarations removeKey: oldName.
    self binding: oldBinding removedFrom: self.
    " re-route now undeclared oldBinding "
    oldBinding value: aClass.

That means that we have an Undeclared #MethodeContext => Context.

And what happens when we ask to remove MethodContext class?
We start removing Context!!!

So what might be necessary is to purgeUndeclared in Kernel-eem.1078 preamble.
And it will work if and only if we removed all MethodContext references.










Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier
Ah, but now that I've put a
    (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self halt: 'debug me'].
I can't reproduce the problem related to removal of Context methods!!!

Instead, I've got the 'Merging Kernel-eem.1078' window with all the MethodContext->Context renaming in conflict
I already reported that in another thread where I told that the upgrade went "smoothly" for both my 32 & 64 bits images.
I don't know if it's related to package cache, or .mcd, or ???

It might also be that package ancestry is not correct du to overwriting of some packages, otherwise we should not get a merge window, but well...

Ah, but now I think I understand: it's just because I have pending changes in Kernel due to the addition of halt: above (same for my images which usually have some unpublished experiments).
We then trigger a merge rather than a load. And the merge sees conflict probably because of broken ancestry (the GUID are not corresponding), but once resolved manually, it processes smoothly without removing Context methods...

I can't instrument code that easily... This will have to wait for another time slot :(

2017-04-08 10:47 GMT+02:00 Nicolas Cellier <[hidden email]>:
My finding today is that ContextPart still have a subclass named Context after update 406.

ContextPart allSubclasses collect: #superclass.
-> an OrderedCollection(ContextPart InstructionStream)

So I will put a conditional breakpoint in removeSelector: to identify the path that remove the selector and re-launch the update

2017-04-08 0:45 GMT+02:00 Nicolas Cellier <[hidden email]>:
Ah no, the binding #MethodContext=>Context is still in the Smalltalk globals declarations (and bindings) after update 406.
It's not at all in Undeclared. Why is that? Oh, no, it's the bootstrapContext method:

    Smalltalk at: #MethodContext ifAbsentPut: [Smalltalk classNamed: #Context].

And there are still plenty references to this binding at this stage.

So why loading Kernel-eem.1078 manually doesn't do any arm, while loading update-eem.407.mcm does?
This time it'll be for tomorrow


2017-04-08 0:21 GMT+02:00 Nicolas Cellier <[hidden email]>:
Ah, but thanks to my previous WeakIdentityDictionary changes, Undeclared holds weakly to its binding.
And after update 406, the references to MethodContext have already vanished.
So maybe a simple garbageCollect might help update 407 to proceed without any problem?

I've just tried that in a package above Kernel in the configuration map....
But it does not seems to work. I'll see tomorrow.

2017-04-07 23:07 GMT+02:00 Nicolas Cellier <[hidden email]>:
Hi,
I tried updating a relatively recent image from:

http://files.squeak.org/6.0alpha/Squeak6.0alpha-16548-32bit/

What happens to me is an unhandled exception MNU Context>>terminateTo: during update 407.

Update 407 is loading Kernel.eem.1078 which is storing the new Kernel classes (Context rather than MethodContext)

Last change written in the change log is:

    Context removeSelector: #terminateTo:

Oh, but there's no such instruction in Kernel-eem.1078...
So what happens exactly which told the image to remove some just installed Context methods???

What happens before that is the boostrapContext in Kernel-eem.1077 preamble

    Smalltalk renameClassNamed: #MethodContext as: #Context

And what happens when we rename is in Environment>>renameClass:from:to:

    oldBinding := self declarationOf: oldName.
    declarations removeKey: oldName.
    self binding: oldBinding removedFrom: self.
    " re-route now undeclared oldBinding "
    oldBinding value: aClass.

That means that we have an Undeclared #MethodeContext => Context.

And what happens when we ask to remove MethodContext class?
We start removing Context!!!

So what might be necessary is to purgeUndeclared in Kernel-eem.1078 preamble.
And it will work if and only if we removed all MethodContext references.











Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

David T. Lewis
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:

> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier


2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.

Ah yes, this time it worked for me.
Someone want to try and confirm?



Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Eliot Miranda-2
Hi Nicolas,

On Apr 8, 2017, at 8:56 AM, Nicolas Cellier <[hidden email]> wrote:



2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.

Ah yes, this time it worked for me.
Someone want to try and confirm?

I just updated a 64-bit image from 16987 to 17157 so I think you have it fixed.  Brilliant!

How do we capture your insights in the code?  I'll try and read the thread and the code and see that comments in the code capture things.  But not today; it's raining and we're going to the beach :-). (Muir beach and the Pelican In are fun in the rain too).

_,,,^..^,,,_ (phone)


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

David T. Lewis
On Sat, Apr 08, 2017 at 09:41:37AM -0700, Eliot Miranda wrote:

> Hi Nicolas,
>
> > On Apr 8, 2017, at 8:56 AM, Nicolas Cellier <[hidden email]> wrote:
> >
> >
> >
> > 2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:
> >>
> >>
> >> 2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:
> >>>
> >>>
> >>> 2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
> >>>> On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> >>>> > Ah, but now that I've put a
> >>>> >     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> >>>> > halt: 'debug me'].
> >>>> > I can't reproduce the problem related to removal of Context methods!!!
> >>>> >
> >>>> > Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> >>>> > MethodContext->Context renaming in conflict
> >>>> > I already reported that in another thread where I told that the upgrade
> >>>> > went "smoothly" for both my 32 & 64 bits images.
> >>>> > I don't know if it's related to package cache, or .mcd, or ???
> >>>> >
> >>>> > It might also be that package ancestry is not correct du to overwriting of
> >>>> > some packages, otherwise we should not get a merge window, but well...
> >>>> >
> >>>> > Ah, but now I think I understand: it's just because I have pending changes
> >>>> > in Kernel due to the addition of halt: above (same for my images which
> >>>> > usually have some unpublished experiments).
> >>>> > We then trigger a merge rather than a load. And the merge sees conflict
> >>>> > probably because of broken ancestry (the GUID are not corresponding), but
> >>>> > once resolved manually, it processes smoothly without removing Context
> >>>> > methods...
> >>>> >
> >>>> > I can't instrument code that easily... This will have to wait for another
> >>>> > time slot :(
> >>>> >
> >>>>
> >>>> Nicolas,
> >>>>
> >>>> Thanks very much for working on this and for posting your results so far.
> >>>> This is not an easy problem to solve.
> >>>>
> >>>> Dave
> >>>>
> >>>>
> >>>
> >>> Hi Dave,
> >>> don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
> >>> So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.
> >>>
> >>> It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.
> >>>
> >>> There's a bunch of removals, starting with:
> >>>
> >>> an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
> >>> a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
> >>> a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
> >>> a MCMethodDefinition(MethodContext>>top)
> >>> a MCMethodDefinition(MethodContext>>terminateTo:) ...
> >>>
> >>> So yes, this mcd is going to shoot the update process.
> >>> Why is it different from the load or merge triggered manually?
> >>> Can't we avoid all the method removals and keep only the class removal?
> >>>
> >>> I can try this snippet in the MCPackageLoader to reject some superfluous method removals:
> >>>
> >>> |  removedClasses |
> >>> removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
> >>> removedClasses addAll: (removedClasses collect: #class).
> >>> removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]
> >>>
> >>> Is it OK as a general change in MC?
> >>>
> >>> Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...
> >>>
> >>> So two more hacks might be necessary:
> >>>
> >>> [Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].
> >>>
> >>> ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).
> >>>
> >>> Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
> >>> However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
> >>> Not sure if it is really necessary...
> >>>
> >>> Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.
> >>>
> >>> ...snip...
> >>> UndefinedObject>>handleSignal:
> >>> Context(nil)>>handleSignal:
> >>> Context(nil)>>handleSignal:
> >>> Context(nil)>>handleSignal:
> >>> Context(nil)>>handleSignal:
> >>>
> >>> I will retry with Bert's hack.
> >>> This update hickup is a tough one.
> >>
> >> And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
> >> Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
> >> I'm going to try this.
> >
> > Ah yes, this time it worked for me.
> > Someone want to try and confirm?
>
> I just updated a 64-bit image from 16987 to 17157 so I think you have it fixed.  Brilliant!
>
> How do we capture your insights in the code?  I'll try and read the thread and the code and see that comments in the code capture things.  But not today; it's raining and we're going to the beach :-). (Muir beach and the Pelican In are fun in the rain too).
>
> _,,,^..^,,,_ (phone)
>

Confirming also. I updated a 64-bit image from 17086 to 17157, no problems.

Fantastic work Nicolas!

Dave


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Eliot Miranda-2
In reply to this post by Nicolas Cellier
Hi Nicolas,

On Sat, Apr 8, 2017 at 8:56 AM, Nicolas Cellier <[hidden email]> wrote:


2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.

Ah yes, this time it worked for me.
Someone want to try and confirm?

There is one wrinkle.  When one updates an image that has been manually updated past Kernel-em.1078 then the preamble script in Squeak-Version fails:

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])

The instVarAt: 6 put: fails because COntextPart is Undeclared and nil.  So better to write it as

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
ContextPart ifNil: [^self].
ContextPart isBehavior ifFalse: [^self].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])



(And anyone else hitting this just use "return entered value" with nil or self to proceed.

_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Nicolas Cellier


2017-04-08 18:48 GMT+02:00 Eliot Miranda <[hidden email]>:
Hi Nicolas,

On Sat, Apr 8, 2017 at 8:56 AM, Nicolas Cellier <[hidden email]> wrote:


2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.

Ah yes, this time it worked for me.
Someone want to try and confirm?

There is one wrinkle.  When one updates an image that has been manually updated past Kernel-em.1078 then the preamble script in Squeak-Version fails:

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])

The instVarAt: 6 put: fails because COntextPart is Undeclared and nil.  So better to write it as

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
ContextPart ifNil: [^self].
ContextPart isBehavior ifFalse: [^self].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])



(And anyone else hitting this just use "return entered value" with nil or self to proceed.

_,,,^..^,,,_
best, Eliot



Hi Eliot, good idea, that should be fixed now.



Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Ken G. Brown
On macOS Sierra 10.12.5 Beta (16F43c), successfully ran through Update Squeak to 17158 without errors on fresh image Squeak6.0alpha-17086-64bit.zip, downloaded from http://files.squeak.org/6.0alpha/Squeak6.0alpha-17086-64bit/ running on 
on opensmalltalk-vm CocoaFast.app from cog_macos64x64_squeak.cog.spur_201701281910.tar from https://github.com/OpenSmalltalk/opensmalltalk-vm/releases

Yay guys!…

Ken G. Brown


On Apr 8, 2017, at 12:24, Nicolas Cellier <[hidden email]> wrote:



2017-04-08 18:48 GMT+02:00 Eliot Miranda <[hidden email]>:
Hi Nicolas,

On Sat, Apr 8, 2017 at 8:56 AM, Nicolas Cellier <[hidden email]> wrote:


2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.

Ah yes, this time it worked for me.
Someone want to try and confirm?

There is one wrinkle.  When one updates an image that has been manually updated past Kernel-em.1078 then the preamble script in Squeak-Version fails:

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])

The instVarAt: 6 put: fails because COntextPart is Undeclared and nil.  So better to write it as

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
ContextPart ifNil: [^self].
ContextPart isBehavior ifFalse: [^self].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])



(And anyone else hitting this just use "return entered value" with nil or self to proceed.

_,,,^..^,,,_
best, Eliot



Hi Eliot, good idea, that should be fixed now.





Reply | Threaded
Open this post in threaded view
|

Re: broken updateStream

Ken G. Brown
In case this means something to someone, running Update Squeak a second time on the already updated to 17158 image, results in update-eem.413.mcm downloading and doing: 
========== Compiler-eem.341 ==========

Add the refactored encoder-specific method generator.  This one moves generation from MethodNode to BytecodeEncoder and subclasses, and hence allows easier bytecode set selection, or at least far more sends to self than to encoder.  Add the MethodNode>>primitive accessor it requires.

==========  Update completed:  17158 -> 17158 ==========

Ken G. Brown

On Apr 8, 2017, at 18:22, Ken G. Brown <[hidden email]> wrote:

On macOS Sierra 10.12.5 Beta (16F43c), successfully ran through Update Squeak to 17158 without errors on fresh image Squeak6.0alpha-17086-64bit.zip, downloaded from http://files.squeak.org/6.0alpha/Squeak6.0alpha-17086-64bit/ running on 
on opensmalltalk-vm CocoaFast.app from cog_macos64x64_squeak.cog.spur_201701281910.tar from https://github.com/OpenSmalltalk/opensmalltalk-vm/releases

Yay guys!…

Ken G. Brown


On Apr 8, 2017, at 12:24, Nicolas Cellier <[hidden email]> wrote:



2017-04-08 18:48 GMT+02:00 Eliot Miranda <[hidden email]>:
Hi Nicolas,

On Sat, Apr 8, 2017 at 8:56 AM, Nicolas Cellier <[hidden email]> wrote:


2017-04-08 17:29 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 17:14 GMT+02:00 Nicolas Cellier <[hidden email]>:


2017-04-08 14:09 GMT+02:00 David T. Lewis <[hidden email]>:
On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
> Ah, but now that I've put a
>     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
> halt: 'debug me'].
> I can't reproduce the problem related to removal of Context methods!!!
>
> Instead, I've got the 'Merging Kernel-eem.1078' window with all the
> MethodContext->Context renaming in conflict
> I already reported that in another thread where I told that the upgrade
> went "smoothly" for both my 32 & 64 bits images.
> I don't know if it's related to package cache, or .mcd, or ???
>
> It might also be that package ancestry is not correct du to overwriting of
> some packages, otherwise we should not get a merge window, but well...
>
> Ah, but now I think I understand: it's just because I have pending changes
> in Kernel due to the addition of halt: above (same for my images which
> usually have some unpublished experiments).
> We then trigger a merge rather than a load. And the merge sees conflict
> probably because of broken ancestry (the GUID are not corresponding), but
> once resolved manually, it processes smoothly without removing Context
> methods...
>
> I can't instrument code that easily... This will have to wait for another
> time slot :(
>

Nicolas,

Thanks very much for working on this and for posting your results so far.
This is not an easy problem to solve.

Dave


Hi Dave,
don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.

It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.

There's a bunch of removals, starting with:

an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
a MCMethodDefinition(MethodContext>>top)
a MCMethodDefinition(MethodContext>>terminateTo:) ...

So yes, this mcd is going to shoot the update process.
Why is it different from the load or merge triggered manually?
Can't we avoid all the method removals and keep only the class removal?

I can try this snippet in the MCPackageLoader to reject some superfluous method removals:

|  removedClasses |
removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
removedClasses addAll: (removedClasses collect: #class).
removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]

Is it OK as a general change in MC?

Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...

So two more hacks might be necessary:

[Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].

ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).

Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
Not sure if it is really necessary...

Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.

...snip...
UndefinedObject>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:
Context(nil)>>handleSignal:

I will retry with Bert's hack.
This update hickup is a tough one.

And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
I'm going to try this.

Ah yes, this time it worked for me.
Someone want to try and confirm?

There is one wrinkle.  When one updates an image that has been manually updated past Kernel-em.1078 then the preamble script in Squeak-Version fails:

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])

The instVarAt: 6 put: fails because COntextPart is Undeclared and nil.  So better to write it as

DoIt
Smalltalk globals unbind: #MethodContext.
[Undeclared at: #MethodContext put: Context copy]
on: AttemptToWriteReadOnlyGlobal
do: [:t1 | t1 resume: true].
ContextPart ifNil: [^self].
ContextPart isBehavior ifFalse: [^self].
^ ContextPart
instVarAt: 6
put: (ContextPart subclasses
select: [:t1 | t1 superclass = ContextPart])



(And anyone else hitting this just use "return entered value" with nil or self to proceed.

_,,,^..^,,,_
best, Eliot



Hi Eliot, good idea, that should be fixed now.