5

I have a Bean configured as follow:

    @Bean(name = "myStopWatch")
    @Scope(value = "prototype")
    public MyStopWatch myStopWatch() {
       return new MyStopWatch();
    }

And MyStopWatch class is as follow:

import org.springframework.util.StopWatch;

public class MyStopWatch {

   private StopWatch sw = new StopWatch();

   public void start() {
       if(!sw.isRunning()) {
           sw.start();
       }
   }

   public void stop() {
       if(sw.isRunning()) {
           sw.stop();
       }
   }
}

We're using this bean in a highly concurrent environment. If my understanding is correct, MyStopWatch class should never be shared bewtween thread, right?

However, we sometimes (very rarely) get the following error:

java.lang.IllegalStateException: Can't start StopWatch: it's already running at org.springframework.util.StopWatch.start(StopWatch.java:127) at org.springframework.util.StopWatch.start(StopWatch.java:116)

So far we couldn't reproduce this behavior with our test. I'm looking for more information regarding how I should correctly define my sw variable (or ma bean) in order to avoid this error.

hublo
  • 1,010
  • 2
  • 12
  • 33
  • What steps are you undertaking to avoid sharing of `MyStopWatch` objects among the threads? – laune Dec 15 '17 at 09:55
  • I thought that having the bean set with prototype scope should be enough. – hublo Dec 15 '17 at 10:06
  • There you are: the "effects" during production runs are most likely due to a race condition. There's no guarantee that there is no thread rescheduling between `sw.isRunning()` and `sw.start()`. – laune Dec 15 '17 at 12:14

2 Answers2

1

Your assumption is wrong. Declaring a bean as prototype does not ensure thread safety. Please see this answer for more details.

As far as using Spring's StopWatch goes, the documentation states that it is not designed to be thread-safe and is also not meant to be used in production.

Sync
  • 3,571
  • 23
  • 30
0

First, it's not thread safe, documentation says so. Having your beans as prototype does not mean that there can't be multiple threads updating it concurrently, it just means that when you ask for such a bean - a new instance is returned all the time (when using getBean or @Autowired for example).

If you want to measure elapsed time (how much it took to do something), stay off the StopWatch from Spring. If you have guava it has a StopWatch too - use that (it's still not thread-safe), otherwise a simple System.nanoTime inside each portion of the code you want to profile would be enough too.

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • What would be the benefits of using guava's `StopWatch' if that is not thread-safe as well? – Sync Dec 15 '17 at 11:53
  • @Synch it's using `System.nanoTime` underneath – Eugene Dec 15 '17 at 11:53
  • @Eugene Ok thanks, but still, as mentioned by Synch, I still don't see the benefit of using the stopwatch from Guava. This wouldn't avoid having to implement a threadsafe solution ourselves. – hublo Dec 15 '17 at 12:18
  • @hublo right, but you are also missing the point here with your `MyStopWatch`. why do you need to extends it? and why expose two methods that guava for example already has. simply put those statements inside the method itself `public void test(){ StopWatch st = st.createStarted() ... do your stuff .. log st.elapsedTime... }` – Eugene Dec 15 '17 at 12:23
  • @Eugene I'm not really extending it, but wrapping it around within a class which is responsible to provide several metrics. The reason why is because we do have several services sharing the same code insfrastructure, and we want to centralize the collect of metrics. Therefore, actually my problem is more to make this stuff thread safe rather than switching to another library for which I'll have to actually solve the same problem as well. I mean, even if I use the Guava SW, i'll have to wrap it within my class, and will encoutner the same problematic. – hublo Dec 15 '17 at 13:07
  • @hublo so you want to measure *not* some method, but some *methods* together? to give you a view of what is going on across multiple calls? – Eugene Dec 15 '17 at 13:08
  • @Eugene yep, this is a very good definition of what we're doing. – hublo Dec 15 '17 at 13:28
  • @hublo thats still somehow questionable, you could bind a stopwatch to a thread via threadlocal for example, but u would need to dispose that too after a cartain call. – Eugene Dec 15 '17 at 13:48
  • @hublo just may be there is a "startig method" that you could do `System.nanoTime` of, that will in turn call all the methods u are interested in. Notice that it seems u dont want thread safety here but more per thread timing, it seems to me. Unless of course you do some async or rx or whatever that will dispatch your calls to some other thread, in which case this will not work – Eugene Dec 15 '17 at 13:53