CODEWITHSUNDEEP
javamultithreadingtimertask
stackoverflow
stackoverflow

Reputation: 19434

Does everything scoped within TimerTask need to be thread safe?

Does everything scoped within TimerTask need to be thread safe?

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?

Upvotes: 4

Views: 2559

Answers (4)

Michael P
Michael P

Reputation: 2067

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

Upvotes: 0

Rahul Ramamoorthy
Rahul Ramamoorthy

Reputation: 31

"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

Upvotes: 0

Fabian
Fabian

Reputation: 363

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.

Upvotes: 4

Nitsan Wakart
Nitsan Wakart

Reputation: 2909

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.

Upvotes: 2

Related Questions