Re: Conversation in RuntimeWireInvoker and thread safety

[Greg Dritschler <gr...@gmail.com>]

Simon,

Yes you are right.  RuntimeWireInvoker is using member variables for
conversation-related data which isn't thread-safe.  I noticed a few other
things too:

1)  Both RuntimeWireInvoker and JavaImplementationInvoker try to manage a
conversational scope object.  In particular they both try to remove the
object at the end of a conversation.  JavaImplementationInvoker gets the
first crack at it and in the process nullifies the conversation id in the
"to" parameters.  I think this prevents
RuntimeWireInvoker.conversationPostInvoke from doing its thing.  This might
leave a conversation dangling in the ConversationManager since only
RuntimeWireInvoker interacts with the ConversationManager.  I think only one
class should be trying to manage conversational objects and it makes sense
to me for it to be the RuntimeWireInvoker (which should cover all
implementations).  In that case the conversation management in
JavaImplementationInvoker should be removed.  In order for this to work
though all bindings need to dispatch through the RuntimeWireInvoker;  I
don't think all do.

2)  Why do we need both a ConversationalScopeContainer and a
ConversationManager to track conversations?  This seems overly complicated
to me.  I think all uses of ConversationManager should be removed and the
code should be updated to work directly with the
ConversationalScopeContainer.  This affects JDKInvocationHandler as well as
RuntimeWireInvoker.

3) RuntimeWireInvoker has a method handleCallback which was copied from
JDKInvocationHandler.  This method makes no sense to me in an inbound
dispatch context.

Greg Dritschler

On Oct 18, 2007 10:32 AM, Simon Laws <si...@googlemail.com> wrote:

> Hi
>
> Similar code for handling conversational state as appears in the
> JDKInvocationHandler has been added to the RuntimeWireInvoker and I was
> just
> putting a change in and spotted something that looks a little strange. I'm
> expecting the RuntimeWireInvoker to be used by many different
> conversations
> as it's created on a wire. This is slightly different from the
> JDKInvocationHandler as in that case you always get the same conversation
> when you send messages through a reference. So I want to fix it so that
> that
> the conversation object is retrieved from the conversation manager each
> time
> before it is used. There are a couple of places where this doesn't happen
> (including in a piece of code I've just edited).But I want to check I
> understand RuntimeWireInvoker correctly before making the change.
>
> Regards
>
> Simon
>

Re: Conversation in RuntimeWireInvoker and thread safety

[Simon Nash <na...@hursley.ibm.com>]

Greg Dritschler wrote:
> Simon,
> 
> Yes you are right.  RuntimeWireInvoker is using member variables for
> conversation-related data which isn't thread-safe.
 >
This seems easy to fix.  In conversationPreInvoke() and
conversationPostInvoke(), the instance variable "conversation" could
be replaced by a local variable "conversation".  In handleCallback(),
the conversationID could be obtained from the "To" endpoint instead
of from the "conversation" variable, or the whole method could be
removed as suggested below.

 >                                                     I noticed a few other
> things too:
> 
> 1)  Both RuntimeWireInvoker and JavaImplementationInvoker try to manage a
> conversational scope object.  In particular they both try to remove the
> object at the end of a conversation.  JavaImplementationInvoker gets the
> first crack at it and in the process nullifies the conversation id in the
> "to" parameters.  I think this prevents
> RuntimeWireInvoker.conversationPostInvoke from doing its thing.  This might
> leave a conversation dangling in the ConversationManager since only
> RuntimeWireInvoker interacts with the ConversationManager.  I think only one
> class should be trying to manage conversational objects and it makes sense
> to me for it to be the RuntimeWireInvoker (which should cover all
> implementations).  In that case the conversation management in
> JavaImplementationInvoker should be removed.  In order for this to work
> though all bindings need to dispatch through the RuntimeWireInvoker;  I
> don't think all do.
> 
The current code should work, because the conversation.end() call in
RuntimeWireInvoker will still happen even if the conversationID in
the "To" parameter was nullified by JavaImplementationInvoker.
However, I agree that it would be good to do all of this processing
in RuntimeWireInvoker as you suggest.

> 2)  Why do we need both a ConversationalScopeContainer and a
> ConversationManager to track conversations?  This seems overly complicated
> to me.  I think all uses of ConversationManager should be removed and the
> code should be updated to work directly with the
> ConversationalScopeContainer.  This affects JDKInvocationHandler as well as
> RuntimeWireInvoker.
> 
I'll defer to SimonL or Raymond on this.  They are more aware of the
reasons why ConversationManager was created.

> 3) RuntimeWireInvoker has a method handleCallback which was copied from
> JDKInvocationHandler.  This method makes no sense to me in an inbound
> dispatch context.
> 
Yes, there was some code copied over from JDKInvocationHandler when
RuntimeWireInvoker was created that makes no sense here and wouldn't
work if it were called.  It might be just handleCallback() or it
might be other code as well.  I'll investigate this further and post
my findings.

   Simon

> Greg Dritschler
> 
> On Oct 18, 2007 10:32 AM, Simon Laws <si...@googlemail.com> wrote:
> 
> 
>>Hi
>>
>>Similar code for handling conversational state as appears in the
>>JDKInvocationHandler has been added to the RuntimeWireInvoker and I was
>>just
>>putting a change in and spotted something that looks a little strange. I'm
>>expecting the RuntimeWireInvoker to be used by many different
>>conversations
>>as it's created on a wire. This is slightly different from the
>>JDKInvocationHandler as in that case you always get the same conversation
>>when you send messages through a reference. So I want to fix it so that
>>that
>>the conversation object is retrieved from the conversation manager each
>>time
>>before it is used. There are a couple of places where this doesn't happen
>>(including in a piece of code I've just edited).But I want to check I
>>understand RuntimeWireInvoker correctly before making the change.
>>
>>Regards
>>
>>Simon
>>
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: tuscany-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: tuscany-dev-help@ws.apache.org