jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vladimir Sitnikov <sitnikov.vladi...@gmail.com>
Subject Re: Why do Functions that only have values as instance variable synchronize execute ?
Date Thu, 16 Oct 2014 21:21:49 GMT
> know wether CompoundVariables
>which are in AbstractFunction subclasses values

I am looking at commit e9228ccf8cca9b130c8730771693cbda024f8bb0
git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1632410 and
it reads that CompoundVariable does not extend AbstractFunction.

>Today with synchronized we ensure that:
>- While execute is being called by one thread, setParameters and other
>execute() (from other threads) are blocked.

I hope we can forbid setParameters usage after JMeterThread start.
I understand bad things can happen if setParameters and execute are
performed concurrently.
I have very little idea how JMeter works internally, however I hope it does
not do such things.

>And due to the fact that it seems we never enter this condition of
>CompoundVariable#execute()  which alters an instance variable :

1) If the CV in question is dynamic (in terms of isDynamic==true after each
execution), then execute is thread safe modulo thread safety of embedded
functions/variables.

2) If the CV in question is not dynamic (e.g. it does not contain
Functions/SimpleVariable), then CV is NOT thread-safe: isDynamic=false &
permanentResults =... are not ordered, thus if multiple threads execute
CV.execute concurrently, then a thread can observe isDynamic==false and
stale value in permanentResults (== it would return wrong results)

I see the following options to fix that (in order of my preference):
2.1) move "isDynamic" initialization to setParameters. It looks like it is
just a instanceof check. If we can't calculate initial value in
setParameters method (i.e. we must compute the value at the first execution
of execute method), then initial value of permanentResults can be set to
null and the check in execute() should read "if (isDynamic ||
permanentResults==null)".

2.2) You can make isDynamic volatile and write code as
"permanentResults=results.toString();
isDynamic=false" (in this order!). This is a minimal change.


PS. I would replace LinkedList compiledElements with ArrayList: it is
faster, more memory efficient, etc and I would use indexed iteration in
execute (to avoid creation of Iterator). It does not hurt readability in
that particular method from my point of view.


​Vladimir

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