Java

Java

Made by DeepSource

synchronized block is empty JAVA-W0151

Anti-pattern
Major
cwe-585

The code contains an empty synchronized block. This could confuse readers of this code later.

An empty synchronized block seems a bit superfluous at first:

synchronized(...) {}

This construct can still be useful though; it can work as a simple write-before-read memory barrier without using volatile variables due to the synchronization guarantees that synchronized blocks provide. However, this way of using synchronized blocks may not be as easily understood as a more explicit mechanism such as a Semaphore.

If this was intended, make sure to document the usage, or rewrite it to make things clearer using abstractions such as Semaphores.

Bad Practice

int variable;
final Object o = new Object();

// ...

new Thread( () ->  // A
{
    // This will become visible to B after the synchronized block.
    variable = 9;
    synchronized( o ) {}

    // ... 
}).start();

new Thread( () ->  // B
{
    // ...
    do {
        synchronized( o ) {}
        // This will pick up the change made in A at some point.
    } while (variable != 9);
    // ...
}).start();

Recommended

The following code with a java.util.conncurrent.Semaphore could be more clear:

int variable;
final Semaphore sem = new Semaphore(0);

// ...


new Thread(() -> { // B
    try {
        // Will wait until A has updated variable's value.
        sem.acquire();
    } catch (Exception e) {
        e.printStackTrace();
    }

    if (var == 9) { ... }
}).start();


new Thread(() -> { // A
    variable = 9;
    // This will be visible within B after sem is released.
    sem.release();
}).start();

References