Is it safe to construct object in following way in multi-threaded environment

Question

public class DataEvent {
    private static final AtomicInteger lastRevision = new AtomicInteger();
    private final int revision;
    private final long threadId;
    private final long timestamp;

    private DataEvent(int revision) {
        this.revision = revision;
        this.threadId = Thread.currentThread().getId();
        this.timestamp = System.nanoTime();
    }

    public static DataEvent newInstance() {
        return new DataEvent(lastRevision.incrementAndGet());
    }
}

My questions are following:

is it absolutely correct to say that all objects will be constructed consistently one by one? I mean that every new object is constructed later then previous one. In other words each new object has timestamp that is bigger then previous one.
how final keyword affects this behavior? As I understand if all object fields are final then it makes constructor atomic in some way. Reference is not published until all final fields are initialized.
what is best practice to construct such objects? Is is enough to make lastRevision atomic or newInstance should be declared as synchronized?

score 2 · Accepted Answer · answered Mar 11 '19 at 15:55

is it absolutely correct to say that all objects will be constructed consistently one by one

No. lastRevision.incrementAndGet() is a blocking call but the scheduler could pause the first thread after receiving the first ID then resume the it after the second thread receives the second ID, meaning both constructors would execute concurrently.

In other words each new object has timestamp that is bigger then previous one.

No, see above.

how final keyword affects this behavior?

It doesn't.

As I understand if all object fields are final then it makes constructor atomic in some way

Incorrect. If every field is final then a class is immutable* which makes it implicitly thread-safe.

what is best practice to construct such objects? Is is enough to make lastRevision atomic or newInstance should be declared as synchronized?

If every instance must be created serially then newInstance should be synchronized. Once it is, there's no point in the atomic integer. Even if you do this, the timestamp might still be the same, depending on the resolution of the underlying system clock.

* well, not exactly. If every field is final and is also immutable itself.

Nestor Sokil · Answer 2 · 2019-03-11T18:12:03.747

The atomicity guarantees are only valid for incrementAndGet() call. So yes, the revisions will be consecutive for every new object, which is the purpose of the atomic datatypes. So, to answer your question: There is no guarantee that the multiple threads will execute the statements within the constructor in the same order as they invoke incrementAndGet(). To achieve this you will have to put this section inside a synchronized block.
finals do not help here. They're purely logical feature which doesn't allow to mutate the field after object creation.
If you really need the timestamps to be aligned with revisions, you have to introduce some form of thread synchronization. You can make the whole method synchronized and then the lastRevision doesn't have to be atomic.

Also, whatever you decide to do you might also want to check on guarantees provided by System.nanoTime() itself. Click.

Is it safe to construct object in following way in multi-threaded environment

2 Answers2