Does everything scoped within TimerTask need to be thread safe?

Question

Example

@Autowired
private MySweetService mySweetService;
int delaySeconds = 0;
int intervalMinutes = 1;    

for(int i=0; i<5; i++) {
    Timer timer = new Timer();

    timer.schedule(new TimerTask() {
        public void run() {
            // This method below is my questionable area
            mySweetService.doStuff(i);
        }
    }, delaySeconds, intervalMinutes);
}

Does everything need to be thread safe within the timerTask anonymous class? Any inherent problem?

I think the `@Autowired` annotation (Spring right?) is not relevant to the question: No matter where the instance comes from, the thread safety issues are relevant. Only thing that might matter is if the service actually changes during the TimerTask lifetime, but thats not what `@Autowired` does (right?). Also to improve the question you might want to change the class name to uppercase. — Fabian, Jun 11 '14 at 22:03

Fabian · Accepted Answer · 2014-06-11T21:55:26.493

4

No. For every Timer there is one thread. The timer will execute only one task at one time. See Timer

Thus the mySweetService.doStuff method does not need to be thread-safe.

Edit: The question was modified. Originally only one Timer instance was created outside the loop, thus there was only one thread. Now it creates theTimers in the loop; the above does no longer hold: Each Timer will have its own thread and race-conditions can occur. This is pretty much equivalent to using Thread or Runnable.

edited Jun 11 '14 at 21:55

answered Jun 11 '14 at 20:40

Fabian

363
2
6

Crap see revision please – stackoverflow Jun 11 '14 at 20:47
In that case my answer is not really helpful to your question anymore. The short answer is: *Yes*, but I think Nitsan Wakart's answer is sufficient. – Fabian Jun 11 '14 at 21:57

Nitsan Wakart · Answer 2 · 2014-06-11T21:03:42.280

2

Right now you have 5 threads that we know about call doStuff, the Timer threads created in the loop. If more than one thread interacts with a method than you have concurrent calls to the same method and you obviously need to consider thread safety.

BUT having a single thread call a method is not enough to provide thread safety. Consider this code:

private int value;
void add(int x) {
  value+=x;
}
void sub(int x) {
  value-=x;
}

Given 2 threads each calling one of these methods the code is obviously not thread safe.

The point is: Thread safety is about state, not about methods.

edited Jun 11 '14 at 21:03

answered Jun 11 '14 at 20:58

Nitsan Wakart

2,841
22
27

There should be five threads. The for loop creates 5 new Timer objects and then runs timer.schedule(new TimerTask().... – stackoverflow Jun 11 '14 at 21:01
Right, than trivially yes you should worry about thread safety(i.e. you need to consider the state)... will edit answer – Nitsan Wakart Jun 11 '14 at 21:02

Rahul Ramamoorthy · Answer 3 · 2018-10-11T14:14:12.183

"If a TimerTask accesses data that is also accessed by other application threads, then not only must the TimerTask do so in a thread safe manner, but so must any other classes that access that data. Often the easiest way to achieve this is to ensure that objects accessed by the TimerTask are themselves thread safe, thus encapsulating the thread safety within the shared objects."-Java Concurrency In Practice-Chapter 1.4

Yes everything inside TimerTask must be Thread safe.In your example, there are 5 threads calling mySweetService.doStuff() concurrently and if there is a non thread safe collection like HashMap it may lead to unexpected behaviour.

As a thumb rule any concurrent call or access to variable or collection must be thread safe.Correct me if I am wrong

"Whether an object needs to be threadsafe depends on whether it will be accessed from multiple threads. This is a property of how the object is used in a program, not what it does. Making an object threadsafe requires using synchronization to coordinate access to its mutable state; failing to do so could result in data corruption and other undesirable consequences" -Java Concurrency In Practice-Chapter 2

Well probably this book has answers to everything :P

score 0 · Answer 4 · answered Oct 11 '18 at 20:19

It is not really clear where else you're using that mySweetService. If you use it in more than one thread, then mySweetService.doStuff(i); has to be thread safe. More accurately, every data structure inside that method has to be properly used in a thread safe manner, and the logic inside has to be thread safe. The simplest way is to make the do Stuff synchronized. But if you use the data inside mySweetService in other methods that are not synchronized, that will also be a problem.

So really what you care about here, is not the logic inside the timer callback, but inside the service itself.

I hope that helps.

Michael, Author of the Concurrency Master course https://www.udemy.com/java-multithreading-concurrency-performance-optimization/?couponCode=CONCURRENCY

Does everything scoped within TimerTask need to be thread safe?

4 Answers4