Issue
Can object having private fields only + all methods synchronized be named as thread safe?
public class Person {
private String name = "John";
private byte age = 30;
private Object somefield;
// synchronized getters and setters
}
Solution
No.
Thread safety as a concept applied to an entire class isn't a boolean proposition (it's not "this is thread safe" and "this is not thread safe"). There is no such thing as a thread safe class. There is only such a thing as a thread safe action.
Here's a trivial example. Let's say you want a hashmap, but 'thread safe' (as in, that the entire object is considered 'thread safe'. Java certainly appears to allow you to create such a beast:
Map<Integer, String> map = Collections.synchronizedMap(new HashMap<Integer, String>());
But.. no. Here's a simple operation that is not thread safe, using the obvious API endpoints to do this, at least, obvious until java 8 which introduced a whole bunch of improvements:
if (!map.containsKey(1)) map.put(1, calculateValueAssociatedWith(1));
This isn't thread safe. After all, perhaps right in the middle (after checking if key 1 is in the map but before adding a mapping for '1'), another thread injects something. Or even, perhaps calculateValueAssociatedWith takes a very long time, and thus 814 threads all arrive at this code within the same 5 minute window or so, they all see that the map does not contain 1, they all start the expensive calculation (813 of those are wasting a ton of CPU cycles and who knows what), and in the end all 814 write the same mapping over and over again into this map. Or worse, let's say the app is inconsistent (broken) if more than one thread works like this.
The problem here is that the actual operation you wanted to perform is: "Calculate a key/value mapping, but.. only if that isn't already in the map - atomically". But that's not what you wrote. That's in fact not even a thing you can write with a single method call, and synchronized only makes thread safe single method calls, not a chain of them, at least, not until java8 (or was it 11?) brought .computeIfAbsent and friends, and I don't even know if they work correctly with synchronizedMap.
The java8 right way to make this operation thread safe (see how you can talk about thread safe operations but not thread safe objects?) is something like this:
synchronized (map) {
if (!map.containsKey(1)) map.put(1, calculateFor(1));
}
More trivially, your Person object isn't final, so anybody can write a subclass that adds a non-synchronized thing, so that's a second reason why it wouldn't be. But let's paper over that and say you did declare it as final, and all fields are private, and all methods that read or write those fields in any way are all marked synchronized. This sounds like one heck of an inefficient take on things, but, let's roll with it: That only works if there is just no notion whatsoever of a 'conjoined operation', where a task can only be performed by invoking more than one method on your Person instance.
Which is never going to happen, hence, the very idea that you would say 'instances are thread safe' is highly misleading.
Unfortunately, various JVMs do have a habit of writing their docs this way. Make no mistake, those are awkward oversimplifications. What they are really attempting to state is that the individual methods are, but not that any operations you do to them are.
Contrast to various classes in the java.util.concurrent package. These aren't just comprised solely of 'thread safe methods', but their entire API is specifically designed for you to perform all operations in a thread safe manner. Partly by having a ton of methods to capture common tasks that you want to be atomic (so not just .containsKey and .put, but also .computeIfAbsent, merge, etc), and partly by having a design that is more fundamentally compatible with being used from multiple threads. For example CopyOnWriteList.
Generally, trying to share state between 2 running threads is intractably complicated. No amount of tossing synchronized keywords at your source files are going to magically make it all disappear, until you get to the logical extreme where at all times, all threads but one are always paused, and your threads are mostly useless now.
The usual solution is to.. not do that. Don't share state between threads casually. Find the few things that truly must be shared and arrange for all reading and writing of this state to go through very particular mechanisms that are highly specialized for the exact task required of them. This can involve using fork/join or picking the right data abstraction in j.u.c (which you should consider as a collection of things that are useful for tasks that come up a lot specifically for multithreaded code that can't avoid sharing state, thus, it's an excellent place to start first) - or a more generalized and much simpler take which is to do all communication through a medium that is designed for it.
Such as databases (which have transactions), or message queues. Why do you think most high-available web service systems do ALL state in databases, leaving absolutely nothing in the server process? It's not just so you can trivially run multiple servers simultaneously. It's because sharing state between connections is much, much simpler this way, and most databases can do a vastly more optimized job at sharing state than the rather blunt tool of 'just block all the things until this is done'.
Answered By - rzwitserloot
Answer Checked By - Katrina (JavaFixing Volunteer)
