jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1490281 - /jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
Date Sun, 09 Jun 2013 13:42:21 GMT
I think I fixed the issue , please review.

Thanks

On Fri, Jun 7, 2013 at 1:40 AM, sebb <sebbaz@gmail.com> wrote:

> On 7 June 2013 00:23, sebb <sebbaz@gmail.com> wrote:
> > On 6 June 2013 20:55, Philippe Mouawad <philippe.mouawad@gmail.com>
> wrote:
> >> Hello sebb,
> >> I don't understand your change.
> >>
> >> What do you mean by:
> >> +            // This class is cloned per thread, and testIterationStart
> is
> >> called from a different thread from samplers
> >> +            // so we need to fetch the implementation separately
> >>
> >> What is the regression introduced by my commit ?
> >
> > There was NPE in HTTPSamplerBase#errorResult on the line
> >
> > e.printStackTrace(new PrintStream(text));
> >
> > because e was null
> >
> >> For me it's the same thread which calls testIterationStart and sample ?
> >
> > No, it's a different thread.
>
> Actually, normally testIterationStart is called by the same thread as
> sampler.
>
> However the test case Bug52310.jmx was failing because of NPE (as
> above) and I think this is caused by sample running in a different
> thread with a different instance.
>
> >> I would like to understand what was wrong in the code I commited.
> >>
> >> Thanks for clarifications.
>
> This problem was first picked up by Jenkins.
>
> >> Regards
> >> Philippe
> >>
> >> On Thu, Jun 6, 2013 at 3:43 PM, <sebb@apache.org> wrote:
> >>
> >>> Author: sebb
> >>> Date: Thu Jun  6 13:43:02 2013
> >>> New Revision: 1490281
> >>>
> >>> URL: http://svn.apache.org/r1490281
> >>> Log:
> >>> Fix bug introduced in http://svn.apache.org/r1489603
> >>> The testIterationStart method is called from a different thread from
> any
> >>> of the samples
> >>>
> >>> Modified:
> >>>
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
> >>>
> >>> Modified:
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
> >>> URL:
> >>>
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java?rev=1490281&r1=1490280&r2=1490281&view=diff
> >>>
> >>>
> ==============================================================================
> >>> ---
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
> >>> (original)
> >>> +++
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
> >>> Thu Jun  6 13:43:02 2013
> >>> @@ -36,8 +36,6 @@ public final class HTTPSamplerProxy exte
> >>>
> >>>      private transient HTTPAbstractImpl impl;
> >>>
> >>> -    private transient Exception initException;
> >>> -
> >>>      public HTTPSamplerProxy(){
> >>>          super();
> >>>      }
> >>> @@ -55,11 +53,14 @@ public final class HTTPSamplerProxy exte
> >>>      /** {@inheritDoc} */
> >>>      @Override
> >>>      protected HTTPSampleResult sample(URL u, String method, boolean
> >>> areFollowingRedirect, int depth) {
> >>> -        if(impl != null) {
> >>> -            return impl.sample(u, method, areFollowingRedirect,
> depth);
> >>> -        } else {
> >>> -            return errorResult(initException, new HTTPSampleResult());
> >>> +        if (impl == null) { // Not called from multiple threads, so
> this
> >>> is OK
> >>> +            try {
> >>> +                impl =
> >>> HTTPSamplerFactory.getImplementation(getImplementation(), this);
> >>> +            } catch (Exception ex) {
> >>> +                return errorResult(ex, new HTTPSampleResult());
> >>> +            }
> >>>          }
> >>> +        return impl.sample(u, method, areFollowingRedirect, depth);
> >>>      }
> >>>
> >>>      // N.B. It's not possible to forward threadStarted() to the
> >>> implementation class.
> >>> @@ -85,17 +86,13 @@ public final class HTTPSamplerProxy exte
> >>>       */
> >>>      @Override
> >>>      public void testIterationStart(LoopIterationEvent event) {
> >>> -        if (impl == null) { // Not called from multiple threads, so
> this
> >>> is OK
> >>> -            try {
> >>> -                impl =
> >>> HTTPSamplerFactory.getImplementation(getImplementation(), this);
> >>> -                initException=null;
> >>> -            } catch (Exception ex) {
> >>> -                initException = ex;
> >>> -            }
> >>> -        }
> >>> -        if(impl != null) {
> >>> +        try {
> >>> +            // This class is cloned per thread, and
> testIterationStart is
> >>> called from a different thread from samplers
> >>> +            // so we need to fetch the implementation separately
> >>> +            HTTPAbstractImpl temp =
> >>> HTTPSamplerFactory.getImplementation(getImplementation(), this);
> >>>              // see
> >>> https://issues.apache.org/bugzilla/show_bug.cgi?id=51380
> >>> -            impl.testIterationStart(event);
> >>> +            temp.testIterationStart(event);
> >>> +        } catch (Exception ex) {
> >>>          }
> >>>      }
> >>>  }
> >>>
> >>>
> >>>
> >>
> >>
> >> --
> >> Cordialement.
> >> Philippe Mouawad.
>



-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message