CODEWITHSUNDEEP
javaconcurrencysynchronized
Daniel Szalay
Daniel Szalay

Reputation: 4101

Is there a synchronization conflict in this given example?

In my code, I use a synchronized block on the resource set inside another synchronized block on the same resource. This did not cause me any problems so far, but I'm curious: what are the consequences if any? Will these synchronized blocks conflict with each other?

Set<myClass> set = Collections.synchronizedSet(new HashSet<myClass>());

synchronized(set) {
    set.add(new myClass());
    ...
    writeSetContentsToFile();
}

public synchronized void writeSetContentsToFile() {
    synchronized(set) {
        ...
    }
}

The set is accessed and changed continously by other threads. I synchronized writeSetContentsToFile(), so there won't be any conflicts at the file resource, and again, there is a synchronized block inside writeSetContentsToFile(), to make sure no changes are made to the set while iterating over it.

Upvotes: 0

Views: 195

Answers (3)

Victor Sorokin
Victor Sorokin

Reputation: 12006

If your operation expressed in

synchronized(set) {
    set.add(new myClass());
    ...
    writeSetContentsToFile();
}

is atomic, encapsulate it in separate class, say, ThreadSafeFileStorableSet under dedicated method, say, synchronized writeContentsToFile().

For example:

class ThreadSafeFileStorableSet {
    private final Set set;

    ThreadSafeFileStorableSet(Set set) {
        this.set = set;
    }

    synchronized void writeSetContentsToFile() {
        // 
    }

    synchronized void addElements(Object[] elems) {
        // 
    }

    private void doWrite() {
        // use instance set
    }
}

Make all other non-private methods of ThreadSafeFileStorableSet class synchronized too, so all operations will be atomic to each other. Then you won't need synchronized (set) snippets inside this class.

This will simplify your code, but may make concurrency level worse. But I don't have enough information to advise on how much worse it could become from reading your code.

Upvotes: 1

artbristol
artbristol

Reputation: 32407

There are two locks here, the one on this, and the one on set. If two threads lock them in different orders, BANG. Deadlock. You haven't shown all your code, so this might not be possible, but what about when someone changes the class to add a new method?

As Peter Lawrey says, you should try to simplify this code.

Upvotes: 1

Peter Lawrey
Peter Lawrey

Reputation: 533530

This did not cause me any problems so far, but I'm curious: what are the consequences if any?

In some ways you answered your own question, it works but its confusing. Its best to simplify the code if you can. You would need to look at the rest of the code to determine whether this is possible.

Upvotes: 1

Related Questions