[Seaside] Seaside and Dynamic variables

[David Shaffer]

Cees De Groot wrote:

>Kilauea, David, Kilauea ;)
>
>But if I understand from your long post, what's wrong with my code -
>it seems to do what you want, no? I am fetching the session from the
>connection pool in my case, but I could just as well read an ivar from
>the session in #withEscapeContinuation and set that in a dynamic
>variable. Every request would then have the same value of the dynamic
>variable, and non-seaside code could be wrapped in non-seaside ways of
>setting up the dynamic variables on the stack. Or even use a different
>method (I've taken to not using dynvars directly, probably inspired by
>WACurrentSession, so I can always change my thoughts - in Kilauea,
>there's XxConnectionPool currentSession for example to get to the
>current db session, which works fine in Seaside now and would be
>trivial to extend to other environments if necessary).
>  
>

I don't know Kilawaklawa (I'm just having fun now :-) very well but I
think there may be problems...

KilaueaConnectionPool>>commit: aBlock
	| sess |
	sess := self current getSession.
	[Bindings clamp: [
		self currentSession: sess.
		^sess commit: aBlock]]
		ensure: [self current releaseSession: sess]

which is sent from:

KilaueaSeasideseSession>>withEscapeContinuation: aBlock 
	^self connectionPoolClass commit: [super withEscapeContinuation: aBlock]


Conceptually:

A new request comes in...you grab a connection from the pool, let's call
it connectionA, and then bind it to the dynamic variable.  On the first
request this works as expected and this dynamic variable is on the
executaion stack and bound to connectionA.  On the next request you do
the same thing, binding connectionB, but this time the invocation of
"sess commit: aBlock" involves evaluating a continuation before
processing callbacks and rendering.  This continuation brings you back
to the stack you were in the _first_ time you executed this code so the
dynamic variable would be bound to the value it had during the first
execution, ie connectionA.  That's how it would work if the "dynamic
variables" were being used in the traditional sense (bound to a stack
frame with lookup going up the call stack).

Now, what I think is going on is that the DynamicBindings package isn't
working as you expect.  My reading of the docs in DynamicBindings says
that if you want to limit a binding to a particular execution stack you
must send it isolate:  Bindings clamp: [Bindings isolate. ...].  So,
what's happening in your case is that your bindings are escaping the
execution stack.  I think this is a big issue.  Try this to see if I'm
right:

1) In a workspace bind the root value of current session to nil:
(KilaueaConnectionPool currentSession: nil)
2) Exercise your Seaside app
3) In a workspace check the root binding of the current session
(KilaueaConnectionPool currentSession) -- do you still see nil?

I think what's happening in your case is that the bindings are leaking
out of the dynamic context and so things _seem_ to be working generally
but if your output in step 3 was non-nil I think you can see that there
is really a problem.  Now, to avoid the leaking you add isolate so...try
adding "Bindings isolate" inside the clamp: and then see if your code is
actually correctly using new sessions (my guess is that it will try to
use an old one).

Here's my take:

1) your code has a bug (lack of isolate) which makes it seem to work by
leaking the bindings up to the root binding
2) fixing that bug would introduce the problem that the new binding is
not available on the call stack

...but I'd be willing to accept that I'm wrong.   One of the things that
makes understanding this difficult is that DynamicBindings have more
complicated lookup semantics than your normal "dynamically bound
variable".  WADynamicVariable has simpler lookup (just goes up the call
stack), try using it just to convince yourself that this pattern is
wrong...the new value of the binding simply isn't on the call stack
after the first invocation.


>That's probably closest to your 2), I think it's the cleanest thing to
>do - make a class-side accessor for the value and let that code decide
>where to grab it from depending on the current environment.
>  
>
Not quite...what I'm suggesting in 2 is something like:

KilaueaConnectionPool>>currentSession
	^WASession current ifNil: [...your binding stuff here...] ifNotNil: [WASession current databaseSession]


That way when you're working in Seaside you're not doing the Bindings
clamp: stuff.  You are only relying on having a dynamic binding for the
session which works fine since the session is available during the first
invocation of withEscapeContinuation:.


>I must say, being able to keep a dynamic binding context between
>requests is sort of cool - thanks for puzzling that out, even though I
>doubt its practical value :)
>  
>

That wasn't my goal.  What I want is a more predictable way to determine
which values will be bound.  The current use of withEscapeContinuation:
for this purpose is fraught with pitfalls...or, I'm just operating on
too little sleep :-)

Obviously some tests which demonstrate the problem are in order here. 
Attached is a simple application which, during withEscapeContinuation:,
increases a global counter and binds its value to a dynamic variable. 
The Seaside component just displays the value of the variable and
provides a link to cause a new request.  What you will notice is that
the binding that the Seaside app sees is always the first one created
when the session was first started.  If you use DynamicBindings and
isolate, you should see similar behavior.

David

-------------- next part --------------
Object subclass: #BPCounter
	instanceVariableNames: ''
	classVariableNames: 'Count'
	poolDictionaries: ''
	category: 'BindingProblems'!

"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

BPCounter class
	instanceVariableNames: ''!

!BPCounter class methodsFor: 'accessing' stamp: 'cds 11/17/2005 10:04'!
count
	^Count := (Count ifNil: [0]) + 1! !


WADynamicVariable subclass: #BPDynamicVariable
	instanceVariableNames: ''
	classVariableNames: ''
	poolDictionaries: ''
	category: 'BindingProblems'!


WASession subclass: #BPSession
	instanceVariableNames: ''
	classVariableNames: ''
	poolDictionaries: ''
	category: 'BindingProblems'!

!BPSession methodsFor: 'responding' stamp: 'cds 11/17/2005 10:06'!
withEscapeContinuation: aBlock 
	| value |
	value := BPCounter count.
	BPDynamicVariable use: value
		during: [^super withEscapeContinuation: aBlock]! !


WAComponent subclass: #BPShowVariable
	instanceVariableNames: ''
	classVariableNames: ''
	poolDictionaries: ''
	category: 'BindingProblems'!

!BPShowVariable methodsFor: 'rendering' stamp: 'cds 11/17/2005 10:08'!
renderContentOn: html
	html text: BPDynamicVariable value.
	html anchorWithAction: [] text: 'next'! !

"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

BPShowVariable class
	instanceVariableNames: ''!

!BPShowVariable class methodsFor: 'class initialization' stamp: 'cds 11/17/2005 10:07'!
initialize
	"self initialize"
	| app |
	app := self registerAsApplication: 'BProblem'.
	app preferenceAt: #sessionClass put: BPSession! !

BPShowVariable initialize!