If I have a volatile boolean (let's call it valid), is the following piece of code thread-safe in Java?
if (valid)
return;
valid = true;
Or, do I need to synchronize since valid is set to true only if it's false (hence set of valid depends on its current value)?
It isn't thread-safe. But if this is the whole code, it wouldn't matter.
This needs to be synchronized, because if one thread evaluates valid as false, and then suspends execution before the assignment, then another one comes along and also checks valid as false, before setting valid to true, you will have two threads running code following on from here (which presumably you don't want).
Edit: An all-around superior alternative would be AtomicBoolean, which uses low-level operations to achieve conditional updates without synchronization.
There are two separate (i.e. non-atomic) accesses to the flag, thus synchronization is necessary unless this thread is the only one doing write operations on the flag. Even then it would probably be good to have synchronization just to be sure in case of future changes.
Use AtomicBoolean. Instances can be simultaneously checked and set.
Your code is not threadsafe, but it really depends on your other code whether or not it is safe to do that.
Do you require that the code following valid = true only be executed once by a single thread? If so, then your code is not safe because any number of threads can read a false value of valid, and then end up setting it to true. For example:
if (valid)
return;
// Imagine every single one of your threads stops and blocks here.
// They will all wake up again and set valid to true and then
// execute the code to follow.
valid = true;
But if you just want to guarantee that the code after valid = true is executed at most one time by any thread... then what you have is fine. But if that's the behavior you require I would achieve that by other means, because using volatile in that case will just look like you don't know what you're doing. For example, you could just not share the valid variable across threads, and allow each thread to just execute the code once.
Also, when in doubt while reasoning about synchronization and volatile... just use synchronization. It is usually more clear, and will get you everything you wanted from using volatile, except it will be easier to reason how the code works.
thread-safety is a system wide property. you cannot look at a piece of code in isolation and call it thread safe/unsafe. it depends on how other threads interact with it and what is the consistency requirement. that being said, most thread-safe design has while() loop instead of if() block, so, your design is most likely incorrect:)