[Seaside-dev] Issue with #removeDecoration:

[John Thornborrow]

Thanks Lukas,

We have implemented a hack in our application which stops the error, but 
we are yet to confirm if it causes memory or other issues as yet, but so 
far we've not noticed any problems. Though my gut feeling tells me it 
will be likely to cause some kind of problem when under heavy/extended use.

WAComponent>>removeDecoration: aDecoration
	| dec |
	dec := self decoration.
	dec = aDecoration ifTrue:
		[self decoration: dec owner.
		^ self].

	"#and: block added to circumvent MNU>>owner problem"
	[dec notNil and: [dec isKindOf: WADecoration]] whileTrue:
		[dec owner = aDecoration
			ifTrue: [dec owner: aDecoration owner.  ^ self]
			ifFalse: [dec := dec owner]]

Many thanks for your time and advice on this.
John

Lukas Renggli wrote:
> Hi John,
> 
>>  I noticed Seaside2.8a1-lr.559 has the comment "fixed the backtracking bug
>> (only an ugly solution)" and was wondering if this is related to the bug
>> below, and also I was wondering what the work around you mentioned is?
> 
> we added (a slightly simplified version) of your test case to the
> functional test-suite of Seaside (WADecorationTask, WADecorationTest).
> The commit comment should have stated that I fixed the test, not the
> actual cause of the problem.
> 
> I tried to explain the problem in my blog (second paragraph):
> 
> 	http://www.lukas-renggli.ch/blog/seaside-sprint-may-08
> 
> Older versions of Seaside did not show that problem, because as soon
> as you registered something for backtracking it was backtracked
> forever. That was one of the reasons why Seaside was so memory hungry
> in versions prior to 2.8. Backtracking only visible components leads
> to strange side effects in rare cases though.
> 
> There are essentially 3 possibilities solving your problem:
> 
> 1. We discussed with Avi and he argues that it is good practice to
> create a new component on every call. Instantiating new components
> might not be desired in your case, but it is certainly what is done in
> most situations. In fact I checked all my Seaside code-base and didn't
> find a single place where this "rule" was violated.
> 
> 2. Make sure that state-snapshots are also takes from invisible
> components (that's how I fixed the test-case).
> 
> WADecorationTask>>updateStates: aSnapshot
> 	super updateStates: aSnapshot.
> 	component1 visiblePresentersDo: [ :each | each updateStates: aSnapshot ].
> 	component2 visiblePresentersDo: [ :each | each updateStates: aSnapshot ]
> 
> 3. Or even simpler, just register the two components as children of the task:
> 
> WADecorationTask>>children
> 	^ Array with: component1 with: component2
> 
> My blog entry sounds a bit too optimistic. The issue is very likely
> not easily fixable in a generic manner and without vasting vast
> amounts of memory. Unless somebody from the community has a flash of
> genius we probably won't be able to change anything there.
> 
> Cheers,
> Lukas
> 

-- 
John Thornborrow
http://www.pinesoft.co.uk


******************************************************************************************************************************************
This email is from Pinesoft Limited. Its contents are confidential to the intended recipient(s) at the email address(es) to which it has been addressed. It may not be disclosed to or used by anyone other than the addressee(s), nor may it be copied in anyway. If received in error, please contact the sender, then delete it from your system. Although this email and attachments are believed to be free of virus, or any other defect which might affect any computer or IT system into which they are received and opened, it is the responsibility of the recipient to ensure that they are virus free and no responsibility is accepted by Pinesoft for any loss or damage arising in any way from receipt or use thereof. *******************************************************************************************************************************************


Pinesoft Limited are registered in England, Registered number: 2914825. Registered office: 266-268 High Street, Waltham Cross, Herts, EN8 7EA