axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chamikara Jayalath <chamikar...@gmail.com>
Subject Re: [Axis2] Possible bug in the AxisEngine.resume() method
Date Thu, 15 Dec 2005 17:28:15 GMT
Hi Glen,

Please see my comments below.

On 12/15/05, Glen Daniels <glen@thoughtcraft.com> wrote:
>
> Hi Chamikara:
>
> +1 and -1 to parts of this patch. :)
>
> First, I agree about merging the chains (op-specifc and global) into one
> - that was the intent (a single EC) from the first Axis2 meetings.  The
> problem with doing it here is that I think this method might get called
> before operation dispatching has occurred (so getAxisOperation() will
> probably give you a null, resulting in a NPE).  The way this should
> probably work is that the act of dispatching to the AxisOperation itself
> (i.e. MessageContext.setAxisOperation()) should cause the chains to be
> merged/replaced.  Then this portion of the code only needs to worry
> about pausing/resuming a single chain.



Actually what I merged was the AxisEngine.send() method (not receive() ).
Shouldn't both chains be available by the time this get called by the
MessageReceiver ?
If so merging in the begining should not be a problem.

For the receive() method, agreed :) . (actually this seems to be the way it
currently work. Dynamic chain replacing has been implemented in the
'checkPostConditions' method of the 'DispatchPhase' class).


Along the lines of making this simpler, I don't like having resumeSend()
> and resumeReceive() - why should the core execution code care which
> direction things are going?  I believe that the correct thing to do here
> is to slightly refactor the chains, and simply have the transport sender
> / message receiver as Handlers on the ends of the execution chain.  That
> way we could remove all the extra logic of calling senders/receivers
> manually, and just have them invoked automatically by executing the
> handler chain (that's how Axis 1 did it too).


+1.  The reason for going for two resume() methods was  the  act of
TransportSender  and  MessageReceiver  being called seperately in the send()
and receive() methods. If they are a part of the execution chain one
resume() method would do it.


So the end result is a single execution chain which pauses/resumes
> simply by indexes, updates itself at operation dispatch, and no extra
> logic to split receiving/sending.
>
> Sound reasonable?


+1


Thanks,
Chamikara



> Chamikara Jayalath wrote:
> > Hi All,
> >
> > There seems to be a bug in the AxisEngine.resume() method (which was
> > recently added)
> >
> > Wihtin this, only the currently attached execution chain is resumed.
> > But  AxisEngine.send() method attachs two execution chains (service
> > specific and global) and invoke both. Because of this if pausing happen
> > in a service specific out handler, the global out handlers are ommitted
> > when resuming. Also resume mothod does not seem to be calling the
> > MessageReceiver or TransportSender.
> >
> > I have attached a fix to this. First doing two msgContext.invoke() calls
> > in the send() method will not work because when resuming we don't know
> > weather the first or second chain we were in when pausing. So I combined
> > both chains to a one ArrayList and invoked at once.
> >
> > Also since we have to call the MessageReceiver or TransportSender after
> > the invocation of handlers (depending on weather we paused in the send()
> > method or receive() method) it seems better to have two resume methods
> > named resumeSend() and resumeReceive() which will call TransportSender
> > and MessageReceiver respectively (after invoking the execution chain).
> >
> > Please see the attached patch.
> >
> > Thank you,
> > Chamikara
> >
> >
> > ------------------------------------------------------------------------
> >
> > Index: engine/AxisEngine.java
> > ===================================================================
> > --- engine/AxisEngine.java    (revision 356796)
> > +++ engine/AxisEngine.java    (working copy)
> > @@ -66,12 +66,13 @@
> >      public void send(MessageContext msgContext) throws AxisFault {
> >          //find and invoke the Phases
> >          OperationContext operationContext =
> msgContext.getOperationContext();
> > -        ArrayList executionChain = operationContext.getAxisOperation
> ().getPhasesOutFlow();
> > -        msgContext.setExecutionChain((ArrayList) executionChain.clone
> ());
> > +        ArrayList outFlow = (ArrayList)
> operationContext.getAxisOperation().getPhasesOutFlow().clone();
> > +        ArrayList globalOutFlow = (ArrayList)
> msgContext.getConfigurationContext
> ().getAxisConfiguration().getGlobalOutPhases().clone();
> > +
> > +        outFlow.addAll(globalOutFlow);
> > +
> > +        msgContext.setExecutionChain(outFlow);
> >          msgContext.invoke();
> > -        msgContext.setExecutionChain
> ((ArrayList)msgContext.getConfigurationContext().
> > -                getAxisConfiguration().getGlobalOutPhases().clone());
> > -        msgContext.invoke();
> >
> >          if (!msgContext.isPaused()) {
> >              //write the Message to the Wire
> > @@ -385,10 +386,29 @@
> >      }
> >
> >
> > -    public void resume(MessageContext msgctx)
> > +    public void resumeSend(MessageContext msgctx)
> >              throws AxisFault {
> >          msgctx.resume();
> > +
> > +        if (!msgctx.isPaused()) {
> > +            //write the Message to the Wire
> > +            TransportOutDescription transportOut =
> msgctx.getTransportOut();
> > +            TransportSender sender = transportOut.getSender();
> > +            sender.invoke(msgctx);
> > +        }
> >      }
> > +
> > +    public void resumeReceive(MessageContext msgctx)
> > +             throws AxisFault {
> > +     msgctx.resume();
> > +
> > +        if (msgctx.isServerSide() && !msgctx.isPaused()) {
> > +            // invoke the Message Receivers
> > +            checkMustUnderstand(msgctx);
> > +            MessageReceiver receiver = msgctx.getAxisOperation
> ().getMessageReceiver();
> > +            receiver.receive(msgctx);
> > +        }
> > +    }
> >
> >      private String getSenderFaultCode(String soapNamespace) {
> >          return SOAP12Constants.SOAP_ENVELOPE_NAMESPACE_URI.equals(
> >
> >
> >
>

Mime
View raw message