You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jmeter-dev@jakarta.apache.org by Jorg Heymans <jh...@domek.be> on 2004/03/17 17:12:42 UTC
Re: Is JMeterContextService.getContext() thread safe? Should it be
such?
Michal Kostrzewa wrote:
> Hello,
>
> I'm investigating strange bug in JMeter HEAD (I'll post bugreport/patch
> soon). I encountered some code I think is wrong. Can you look into the
> class org.apache.jmeter.threads.JMeterContextService?
>
> 1) I suspect it's supposed to be some kind of a singleton (private
> constructor, a method checking if instance is already created and
> ensuring that there is only one instance) but this class doesn't use
> this feature! (??) The only meaningful method (getContext) is static
private method init() is used from getContext() and setContext() no?
>
> 2) But 1) is not the worse - getContext is not synchronized. It
> maintains the map of context indexed by thread names. This map is shared
> to all threads (by the fact it's static) but the access point
> (getContext) is not synchronized.
>
Changing it from HashMap() to HashTable() would solve this.
> Am I wrong in suspecting this class?
Doesn't look like it from the low level side of things. I don't know if
anywhere higher up it is guaranteed that only one thread is accessing
this class at the same time. One of the developers could give you better
insight.
Sorry if i'm being too top-level, just saw this thread and thought i'ld
chip in.
>
> best regards,
> Michal Kostrzewa
Jorg Heymans
---------------------------------------------------------------------
To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
Re: Is JMeterContextService.getContext() thread safe? Should it be
such?
Posted by Michal Kostrzewa <m....@pentacomp.com.pl>.
> The initialization of the contexts is a bit unclean here, suggesting a
> potential problem, but whether there's actually a problem depends on
> whether the test environment ensures things are set up prior to all
the
> threads getting in on things. I'll have to dig a little to determine
> it. In any case, the initialization of the context should be done
> elsewhere and guaranteed to occur before the test starts.
It's entirely true, but the initialization of contexts is not run as it
should (see http://issues.apache.org/bugzilla/show_bug.cgi?id=27744).
CounterConfig gets the context with null JMeterVariables. Moreover, the
behaviour depends on implementing NoThreadClone.
I'm debugging it now, but no luck so far :(
best regards,
Michal Kostrzewa
>
> -Mike
>
> On Wed, 2004-03-17 at 11:55, Michal Kostrzewa wrote:
> > Thank you for response
> >
> > > > 2) But 1) is not the worse - getContext is not synchronized. It
> > > > maintains the map of context indexed by thread names. This map
is shared
> > > > to all threads (by the fact it's static) but the access point
> > > > (getContext) is not synchronized.
> > > >
> > > Changing it from HashMap() to HashTable() would solve this.
> >
> > Well, not exactly in this case. Consider the following code take
from
> > JMeterContextService class:
> >
> > static public JMeterContext getContext()
> > {
> >
> > init();
> >
> > JMeterContext context =
> > (JMeterContext)
> > contextMap.get(Thread.currentThread().getName());
> >
> > if (context == null)
> > {
> > context = new JMeterContext();
> > setContext(context);
> > }
> >
> > return context;
> >
> > }
> >
> > It's the classic 'test and set' problem (checking for the context
and
> > creating one if null).
> >
> > Moreover I think the HashMap is better to use (HashTable is older),
if
> > you want a synchronized version of this, you can use
> >
> > Map m = Collections.synchronizedMap(new HashMap(...));
> >
> >
> > > > Am I wrong in suspecting this class?
> > > Doesn't look like it from the low level side of things. I don't
know if
> > > anywhere higher up it is guaranteed that only one thread is
accessing
> > > this class at the same time. One of the developers could give you
better
> > > insight.
> >
> > I'm afraid it's not guaranteed at higher level, the more I look into
the
> > code the more I'm sure about this :))
> >
> > >
> > > Sorry if i'm being too top-level, just saw this thread and thought
i'ld
> > > chip in.
> >
> > Thank you again :)
> > best regards,
> > Michal Kostrzewa
> >
> >
> >
> >
> >
---------------------------------------------------------------------
> > To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
Re: Is JMeterContextService.getContext() thread safe? Should it be
such?
Posted by Michael Stover <ms...@apache.org>.
The initialization of the contexts is a bit unclean here, suggesting a
potential problem, but whether there's actually a problem depends on
whether the test environment ensures things are set up prior to all the
threads getting in on things. I'll have to dig a little to determine
it. In any case, the initialization of the context should be done
elsewhere and guaranteed to occur before the test starts.
-Mike
On Wed, 2004-03-17 at 11:55, Michal Kostrzewa wrote:
> Thank you for response
>
> > > 2) But 1) is not the worse - getContext is not synchronized. It
> > > maintains the map of context indexed by thread names. This map is shared
> > > to all threads (by the fact it's static) but the access point
> > > (getContext) is not synchronized.
> > >
> > Changing it from HashMap() to HashTable() would solve this.
>
> Well, not exactly in this case. Consider the following code take from
> JMeterContextService class:
>
> static public JMeterContext getContext()
> {
>
> init();
>
> JMeterContext context =
> (JMeterContext)
> contextMap.get(Thread.currentThread().getName());
>
> if (context == null)
> {
> context = new JMeterContext();
> setContext(context);
> }
>
> return context;
>
> }
>
> It's the classic 'test and set' problem (checking for the context and
> creating one if null).
>
> Moreover I think the HashMap is better to use (HashTable is older), if
> you want a synchronized version of this, you can use
>
> Map m = Collections.synchronizedMap(new HashMap(...));
>
>
> > > Am I wrong in suspecting this class?
> > Doesn't look like it from the low level side of things. I don't know if
> > anywhere higher up it is guaranteed that only one thread is accessing
> > this class at the same time. One of the developers could give you better
> > insight.
>
> I'm afraid it's not guaranteed at higher level, the more I look into the
> code the more I'm sure about this :))
>
> >
> > Sorry if i'm being too top-level, just saw this thread and thought i'ld
> > chip in.
>
> Thank you again :)
> best regards,
> Michal Kostrzewa
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
--
Michael Stover <ms...@apache.org>
Apache Software Foundation
---------------------------------------------------------------------
To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
Re: Is JMeterContextService.getContext() thread safe? Should it be
such?
Posted by Michal Kostrzewa <m....@pentacomp.com.pl>.
Thank you for response
> > 2) But 1) is not the worse - getContext is not synchronized. It
> > maintains the map of context indexed by thread names. This map is shared
> > to all threads (by the fact it's static) but the access point
> > (getContext) is not synchronized.
> >
> Changing it from HashMap() to HashTable() would solve this.
Well, not exactly in this case. Consider the following code take from
JMeterContextService class:
static public JMeterContext getContext()
{
init();
JMeterContext context =
(JMeterContext)
contextMap.get(Thread.currentThread().getName());
if (context == null)
{
context = new JMeterContext();
setContext(context);
}
return context;
}
It's the classic 'test and set' problem (checking for the context and
creating one if null).
Moreover I think the HashMap is better to use (HashTable is older), if
you want a synchronized version of this, you can use
Map m = Collections.synchronizedMap(new HashMap(...));
> > Am I wrong in suspecting this class?
> Doesn't look like it from the low level side of things. I don't know if
> anywhere higher up it is guaranteed that only one thread is accessing
> this class at the same time. One of the developers could give you better
> insight.
I'm afraid it's not guaranteed at higher level, the more I look into the
code the more I'm sure about this :))
>
> Sorry if i'm being too top-level, just saw this thread and thought i'ld
> chip in.
Thank you again :)
best regards,
Michal Kostrzewa
---------------------------------------------------------------------
To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org