CODEWITHSUNDEEP
javamultithreadingsynchronize
Vishal Zanzrukia
Vishal Zanzrukia

Reputation: 4973

Java multi-threading confusion

Code Sample 1:

class Test {
    MyObj myObj = new MyObj();

    public void test() {
        // doing my other stuff
        synchronized (myObj) {
            // accessing myObj
        }
    }
}

Code Sample 2:

class Test {
    MyObj myObj = new MyObj();

    public void test() {
        synchronized (myObj) {
            // doing my other stuff

            // accessing myObj
        }
    }
}

Code Sample 3:

class Test {
    MyObj myObj = new MyObj();

    public synchronized void test() {
        // doing my other stuff

        // accessing myObj
    }
}

I want to keep thread safe to myObj in above code snaps. So which of above code snap is more preferable and Why?

Upvotes: 0

Views: 109

Answers (4)

Beri
Beri

Reputation: 11600

Holding a lock on whole method (if it's long) is very costly. Not only it could block whole threat, but also it blocks code that does not need to be synchronized. Secondly sometimes you may not use your omyObj, in such cases example 2 and 3 would be bad.

To be short: synchronize only part of block, where you are working with your object. You can also use volatile on your object, to check for it's value, without synchronize block. Remember locking can be costly.

But when your method is short enough, or contains only code with myObj, then making whole method synchonized is a good solution.

public void test() {
   // doing my other stuff, not concernign myObj
    synchronized (myObj) {
        // do things with myObj (read/write)
    }
   // doing my other stuff, not concernign myObj
}

Look at this singleton example.

Upvotes: 1

Solomon Slow
Solomon Slow

Reputation: 27115

None of your examples contains enough information.

Where is the data? What are the invariants? When/Where/How do threads access the data?

"Thread safety" means nothing without data. The documentation for a stateful class usually will provide guarantees about how its methods will advance the state of an instance. "Thread-safe" means that those guarantees will be met even when the methods are called by many threads. But if the class is not stateful, or if there are no such guarantees, then "thread-safe" means nothing.

P.S.: Code Sample 1 has one important advantage over the others: The mutex (i.e., the synchronized block) is smaller. As a rule of thumb, you want your mutexes to be as small as possible. If "my other stuff" does not care about the data that are protected by the mutex, then it should not happen in the mutex because that would block other threads from accessing the data, and it would block them for no useful reason.

P.P.S.: +1 for @Ordous's comment: There is no way to know whether some object's state is thread-safe or not unless you can see every block of code that can modify or examine the state. Since myObj is not private in your example, we must assume that there is code elsewhere that could modify or examine its state. Since we can't see that code, we must assume that myObj is not thread safe.

Upvotes: 1

ReZa
ReZa

Reputation: 1283

according to codes that i have seen in Open Source Projects, The 1st code sample is better,because when you do not need object any more you release it so other threads can access it.

Upvotes: 1

Sanjay Rabari
Sanjay Rabari

Reputation: 2081

code sample 1 is appropriate. because holding the lock on object and then doing some other stuff is not appropriate.

Upvotes: 3

Related Questions