jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <p.moua...@ubik-ingenierie.com>
Subject Re: Why do Functions that only have values as instance variable synchronize execute ?
Date Thu, 16 Oct 2014 20:01:48 GMT
Hello sebb, Vladimir,
@Vladimir, as you seem to be a Threading expert, how about a critical
review of those functions ? An external eye would be welcome :-)

I think the main issue in this topic is to know wether CompoundVariables
which are in AbstractFunction subclasses values are thread safe (to the
exception of the non thread safe functions).

Today with synchronized we ensure that:
- While execute is being called by one thread, setParameters and other
execute() (from other threads) are blocked.
Currenlty:
- setParameters is called by 1 thread StandardJMeterEngine
- Execute is called by ThreadGroup X-X


If we remove the synchronized from execute then:
- CompoundVariable instance will be called by Many threads at the same time


My analysis is that it should be ok looking at CompoundVariable#execute()
and knowing the instance variables are initialized within
StandardJMeterEngine single thread:
    private boolean hasFunction, isDynamic;
    private String permanentResults = ""; // $NON-NLS-1$
    private LinkedList<Object> compiledComponents = new
LinkedList<Object>();

And due to the fact that it seems we never enter this condition of
CompoundVariable#execute()  which alters an instance variable :
if (!testDynamic) {
            isDynamic = false;
            permanentResults = results.toString();
        }

But this needs to be double checked.

Regards



On Thu, Oct 16, 2014 at 8:15 PM, sebb <sebbaz@gmail.com> wrote:

>
> On 16 October 2014 19:00, Vladimir Sitnikov <sitnikov.vladimir@gmail.com>
> wrote:
> >>I'm not sure I agree with that; the value of the mutable field needs
> >>to be published safely as well.
> >
> > Do I read it properly? "you are not sure if java.lang.String is safe"
>
> No, I meant that Globals.state is not published safely.
>
> > It is safe since java 1.5.
> > This is ensured by "17.5 final fields semantics" was added to JMM.
> >
> >> But Globals.state is not final or volatile.
> >
> > Here's "safe publication" via data race (Globals.state is not
> > volatile/synchronized/etc)
> > Thread 1:
> >   class Globals { public static String state; }
> >   Globals.state = new String(new char[]{'h','e','l','l','o'}); // no
> > volatile fields involved
> >
> > Thread 2:
> >   System.out.println(Globals.state); // this _must_ print either null or
> > "hello".
> >
> > Note how thread 2 must not print h/he/hel/hell/, even though thread 1
> > copies characters from one array to another one.
> >
> >>How does that illustrate what you wrote here: "Just synchronization on
> the
> > same lock is not sufficient"
> >
> > Ok, here's example with synchronizations:
> >
> > Thread 1:
> > Globals.str = new StringFromFile();
> > str.setParameters(...) // this is method is synchronized
> >
> > Thread 2:
> > Globals.str.execute() // this method is synchronized
> >
> > There is no guarantee that execute will observe proper state of
> > StringFromFile.
> > For instance, str.execute might be executed before setParameters, then
> > "setParameters would not happens-before execute", thus there would be no
> > "safe publication from setParameters to execute".
>
> Of course not.
> I never meant to imply that synchronization affects execution order.
>
> >> True, but that would also affect single threaded code.
> >
> > The original question was to avoid contention between multiple threads. I
> > do not think we need spend time on discussing single-threaded executions.
>
> I was merely pointing out that if execute() is invoked before
> setParameters() then execute() won't see the new value in a
> single-threaded context either.
>
> That seemed so obvious that it did not need stating.
> Assuming that the methods are called in the correct sequence, there is
> still the possibility of unsafe publication in the absence of
> volatile/synch/etc when multiple threads are involved.
>
> > Once again my main point: I am not sure if volatile/synchronized is
> > required. I would like to dodge non-required synchronization.
>
> Fair enough.
>
> > Vladimir
>



-- 
Cordialement.
Philippe M.

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