a bug in Glamour
Locked
 
|
Hi,
I noticed that in some complex updating between panels, using an external announcer, some of the update subscriptions were not removed when the browser is closed. I debugged a little and I founded that this implementation: GLMUpdateAction>>unregisterFromAllAnnouncements announcerObjects ifNotNil: [ announcerObjects do: [:each | each unsubscribe: self ] ] is bugged, because if announcerObjects are not previously computed (and in some cases that's what happens), the subscription is not removed. This implementation (just using the accessor instead the direct object), solves the problem (but I don't know is it's a right fix, and it should be a fix in other place) GLMUpdateAction>>unregisterFromAllAnnouncements self announcerObjects ifNotNil: [ :objects | objects do: [:each | each unsubscribe: self ] ] Cheers, Esteban _______________________________________________ Moose-dev mailing list [hidden email] https://www.iam.unibe.ch/mailman/listinfo/moose-dev |
|
|
Excellent catch, Esteban!
I know about this problem since half a year, but I could not figure out where the problem was. I opened a ticket: http://code.google.com/p/moose-technology/issues/detail?id=492 Before integrating the fix, could you provide a test case that fails with the current implementation? Cheers, Doru On 15 Jan 2011, at 14:29, Esteban Lorenzano wrote: > Hi, > I noticed that in some complex updating between panels, using an external announcer, some of the update subscriptions were not removed when the browser is closed. I debugged a little and I founded that this implementation: > > GLMUpdateAction>>unregisterFromAllAnnouncements > announcerObjects ifNotNil: [ > announcerObjects do: [:each | > each unsubscribe: self ] ] > > is bugged, because if announcerObjects are not previously computed (and in some cases that's what happens), the subscription is not removed. > This implementation (just using the accessor instead the direct object), solves the problem (but I don't know is it's a right fix, and it should be a fix in other place) > > GLMUpdateAction>>unregisterFromAllAnnouncements > self announcerObjects ifNotNil: [ :objects | > objects do: [:each | > each unsubscribe: self ] ] > > Cheers, > Esteban > _______________________________________________ > Moose-dev mailing list > [hidden email] > https://www.iam.unibe.ch/mailman/listinfo/moose-dev -- www.tudorgirba.com "In a world where everything is moving ever faster, one might have better chances to win by moving slower." _______________________________________________ Moose-dev mailing list [hidden email] https://www.iam.unibe.ch/mailman/listinfo/moose-dev |
|
|
I forgot to mention that right now we have two tests, but they do not capture the problem:
GLMBrowserTest>>testUnregisterAnnouncements GLMBrowserTest>>testUnregisterAnnouncementsWhenRemovingPane Cheers, Doru On 15 Jan 2011, at 14:41, Tudor Girba wrote: > Excellent catch, Esteban! > > I know about this problem since half a year, but I could not figure out where the problem was. > > I opened a ticket: > http://code.google.com/p/moose-technology/issues/detail?id=492 > > Before integrating the fix, could you provide a test case that fails with the current implementation? > > Cheers, > Doru > > > On 15 Jan 2011, at 14:29, Esteban Lorenzano wrote: > >> Hi, >> I noticed that in some complex updating between panels, using an external announcer, some of the update subscriptions were not removed when the browser is closed. I debugged a little and I founded that this implementation: >> >> GLMUpdateAction>>unregisterFromAllAnnouncements >> announcerObjects ifNotNil: [ >> announcerObjects do: [:each | >> each unsubscribe: self ] ] >> >> is bugged, because if announcerObjects are not previously computed (and in some cases that's what happens), the subscription is not removed. >> This implementation (just using the accessor instead the direct object), solves the problem (but I don't know is it's a right fix, and it should be a fix in other place) >> >> GLMUpdateAction>>unregisterFromAllAnnouncements >> self announcerObjects ifNotNil: [ :objects | >> objects do: [:each | >> each unsubscribe: self ] ] >> >> Cheers, >> Esteban >> _______________________________________________ >> Moose-dev mailing list >> [hidden email] >> https://www.iam.unibe.ch/mailman/listinfo/moose-dev > > -- > www.tudorgirba.com > > "In a world where everything is moving ever faster, > one might have better chances to win by moving slower." > > > -- www.tudorgirba.com "There are no old things, there are only old ways of looking at them." _______________________________________________ Moose-dev mailing list [hidden email] https://www.iam.unibe.ch/mailman/listinfo/moose-dev |
|
|
In reply to this post by Tudor Girba
he, no... I was too optimistic, that solves just part of the problem, not completely :(
Now I'm debugging to try figure out what's happening... once I get it, I will provide a test :) Cheers, Esteban El 15/01/2011, a las 10:41a.m., Tudor Girba escribió: > Excellent catch, Esteban! > > I know about this problem since half a year, but I could not figure out where the problem was. > > I opened a ticket: > http://code.google.com/p/moose-technology/issues/detail?id=492 > > Before integrating the fix, could you provide a test case that fails with the current implementation? > > Cheers, > Doru > > > On 15 Jan 2011, at 14:29, Esteban Lorenzano wrote: > >> Hi, >> I noticed that in some complex updating between panels, using an external announcer, some of the update subscriptions were not removed when the browser is closed. I debugged a little and I founded that this implementation: >> >> GLMUpdateAction>>unregisterFromAllAnnouncements >> announcerObjects ifNotNil: [ >> announcerObjects do: [:each | >> each unsubscribe: self ] ] >> >> is bugged, because if announcerObjects are not previously computed (and in some cases that's what happens), the subscription is not removed. >> This implementation (just using the accessor instead the direct object), solves the problem (but I don't know is it's a right fix, and it should be a fix in other place) >> >> GLMUpdateAction>>unregisterFromAllAnnouncements >> self announcerObjects ifNotNil: [ :objects | >> objects do: [:each | >> each unsubscribe: self ] ] >> >> Cheers, >> Esteban >> _______________________________________________ >> Moose-dev mailing list >> [hidden email] >> https://www.iam.unibe.ch/mailman/listinfo/moose-dev > > -- > www.tudorgirba.com > > "In a world where everything is moving ever faster, > one might have better chances to win by moving slower." > > > > > _______________________________________________ > Moose-dev mailing list > [hidden email] > https://www.iam.unibe.ch/mailman/listinfo/moose-dev _______________________________________________ Moose-dev mailing list [hidden email] https://www.iam.unibe.ch/mailman/listinfo/moose-dev |
|
|
Thanks a lot!
Doru On 15 Jan 2011, at 14:45, Esteban Lorenzano wrote: > he, no... I was too optimistic, that solves just part of the problem, not completely :( > Now I'm debugging to try figure out what's happening... once I get it, I will provide a test :) > > Cheers, > Esteban > > El 15/01/2011, a las 10:41a.m., Tudor Girba escribió: > >> Excellent catch, Esteban! >> >> I know about this problem since half a year, but I could not figure out where the problem was. >> >> I opened a ticket: >> http://code.google.com/p/moose-technology/issues/detail?id=492 >> >> Before integrating the fix, could you provide a test case that fails with the current implementation? >> >> Cheers, >> Doru >> >> >> On 15 Jan 2011, at 14:29, Esteban Lorenzano wrote: >> >>> Hi, >>> I noticed that in some complex updating between panels, using an external announcer, some of the update subscriptions were not removed when the browser is closed. I debugged a little and I founded that this implementation: >>> >>> GLMUpdateAction>>unregisterFromAllAnnouncements >>> announcerObjects ifNotNil: [ >>> announcerObjects do: [:each | >>> each unsubscribe: self ] ] >>> >>> is bugged, because if announcerObjects are not previously computed (and in some cases that's what happens), the subscription is not removed. >>> This implementation (just using the accessor instead the direct object), solves the problem (but I don't know is it's a right fix, and it should be a fix in other place) >>> >>> GLMUpdateAction>>unregisterFromAllAnnouncements >>> self announcerObjects ifNotNil: [ :objects | >>> objects do: [:each | >>> each unsubscribe: self ] ] >>> >>> Cheers, >>> Esteban >>> _______________________________________________ >>> Moose-dev mailing list >>> [hidden email] >>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >> >> -- >> www.tudorgirba.com >> >> "In a world where everything is moving ever faster, >> one might have better chances to win by moving slower." >> >> >> >> >> _______________________________________________ >> Moose-dev mailing list >> [hidden email] >> https://www.iam.unibe.ch/mailman/listinfo/moose-dev > > > _______________________________________________ > Moose-dev mailing list > [hidden email] > https://www.iam.unibe.ch/mailman/listinfo/moose-dev -- www.tudorgirba.com "Not knowing how to do something is not an argument for how it cannot be done." _______________________________________________ Moose-dev mailing list [hidden email] https://www.iam.unibe.ch/mailman/listinfo/moose-dev |
|
|
In reply to this post by Tudor Girba
ok... it is more complicated than I thought... this is what I dug for now (sorry, this is large):
1) the fix proposed originaly is still necesary. 2) GLMBrowser>>unregisterFromAllAnnouncements needs to be defined this way: unregisterFromAllAnnouncements super unregisterFromAllAnnouncements. self panes do: [:each | each presentations unregisterFromAllAnnouncements ]. self transmissions do: [ :each | each destination pane unregisterFromAllAnnouncements ] (this include panes in transmissions in the clean up) 3) this is the ugly part: there is an identity problem, because GLMUpdateActions are being copied a lot of times, to ensure the browser function. And unsubscribe: checks for identical objects (==) to remove from the annoncements. So... and THIS IS NOT THE SOLUTION, just a validation of the problem. I did: a) I added a method to Announcer: Announcer>>unsubscribeNonIdentical: anObject subscriptions ifNil: [ ^ self ]. subscriptions keysAndValuesDo: [ :class :actions | subscriptions at: class put: (actions reject: [ :each | each receiver = anObject ]) ]. subscriptions keysAndValuesRemove: [ :class :actions | actions isEmpty ] (it just changes the check by == to a check by =) b) assign an unique id, = and hash to GLMUpdateAction: id ^id ifNil: [ id = UUID new asString ] = other self species = other species ifFalse: [ ^false ]. ^self id = other id hash ^self species hash bitXor: self id hash c) modify GLMUpdateAction>>unregisterFromAllAnnouncements self announcerObjects do: [:each | each unsubscribeNonIdentical: self ] and with all this changes... announcement un-suscription is working :P this is the test who validates it: GLMUpdateAction>>testUnregisterAnnouncementsWhenUpdatingPane | announcer browser presentation | announcer := Announcer new. browser := GLMTabulator new. browser row: #one. browser transmit to: #one; andShow: [ :presenter | (presentation := presenter list) updateOn: TestAnnouncement from: [ announcer ]; display: [ :start | start to: 10 ] ]. browser startOn: 1. presentation registerAnnouncements. browser unregisterFromAllAnnouncements. self assert: (announcer instVarNamed: 'subscriptions') isEmpty. So... this makes the un-suscription works, but the whole "id" thing is really-really ugly. So, as I don't know much of Glamour, maybe you can say me a real fix for this :) Cheers, Esteban pd: I attached all this as a comment on issue http://code.google.com/p/moose-technology/issues/detail?id=492 El 15/01/2011, a las 10:43a.m., Tudor Girba escribió:
_______________________________________________ Moose-dev mailing list [hidden email] https://www.iam.unibe.ch/mailman/listinfo/moose-dev |
«
Return to Moose
|
1 view|%1 views
| Free forum by Nabble | Edit this page |