[concurrency-interest] concurrency newbie question

Tim Peierls tim at peierls.net
Tue Jun 23 09:40:35 EDT 2009


Excellent advice -- not silly at all!  A real guru would have included this
broader context from the start. ;-)
Minor point: For classes that must be confined to a single thread, rather
than (only) putting a prose description like "thread contained" in the
javadoc comments, consider in addition annotating such classes with
@NotThreadSafe (
http://jcip.net/annotations/doc/net/jcip/annotations/NotThreadSafe.html).

The only way @NotThreadSafe classes can be used correctly is through "serial
thread confinement", i.e., confinement of an object to a single thread at a
time with any hand-off of the object between threads restricted to *
happens-before* relationships such as those described in
http://java.sun.com/javase/6/docs/api/java/util/concurrent/package-summary.html#MemoryVisibility.
(Serial thread confinement trivially includes simple confinement of an
object to a single thread for the lifetime of the object.)

Incidentally, there is no JCiP annotation for "effectively immutable"
classes, in which all mutative operations *happen-before* the object is
published and multiple threads may perform read-only access after (safe)
publication. Maybe there should be one.

--tim

On Mon, Jun 22, 2009 at 11:26 PM, Chris Kessel/Lou Doherty <
chriskessel at verizon.net> wrote:

> Well, one of the gurus answered, so I feel a bit silly answering, but
> here's
> my .02.
>
> First make sure you know how the object is being used. There are a great
> many POJOs that never live beyond the length of a single thread. They're
> loaded from a DB, modified, and saved all within a single thread (as a
> common example). They aren't shared so the concurrency concerns aren't a
> problem. I simply javadoc them as thread contained at the very top so that
> folks know they aren't concurrent safe.
>
> For those items that need thread safety, figure out the concern. Is it just
> publishing that's the issue or do you have multiple threads modifying it?
> I've discovered I can get away with creating immutable classes frequently
> and then, when a mod is needed, create a new immutable instance to replace
> it rather than modifying the original. As long as you aren't creating
> masses
> of objects, that works well. If you've passing a collection around, you can
> get publishing for free with the concurrent safe collections.
>
> It's the rare class that gets really tricky, usually a service with some
> sort of state, like objects in a persistent queue that are going to be
> called asynchronously with listeners.
>
> When I read JCIP, I realize how horribly unsafe all my code I'd ever
> written
> was :), but I also found I didn't need mass changes to my coding approach
> so
> much as to think about how each class might be used from a threading
> viewpoint. From there, only a fairly small set needed thread safety. JCIP
> gave me the tools to understand what thread safe means and how to make
> those
> classes safe.
>
> Chris
>
> -----Original Message-----
> From: concurrency-interest-bounces at cs.oswego.edu
> [mailto:concurrency-interest-bounces at cs.oswego.edu] On Behalf Of Ashley
> Williams
> Sent: Monday, June 22, 2009 12:21 PM
> To: concurrency-interest at cs.oswego.edu
> Subject: [concurrency-interest] concurrency newbie question
>
> Hi,
>
> After reading java concurrency in practice, I've now realize how little I
> know after all these years so I'm trying to put that right. So as a first
> step I would like to make a settings class, which is just a bunch of
> getters
> and setters, a little more threadsafe and so I would appreciate any helpful
> comments.
>
>
> I think I am right in just using a final reference to an atomic wrapper for
> the simple property types, but I'm unsure if I'm approaching the map
> property correctly. In the code below I've tried to follow these guidelines
> for the 'machines' field:
>
> 1. The getter should always return a copy.
>
> 2. The setter should replace the currently held list, rather than trying to
> clear() then putAll() which can cause a race condition if another thread
> gets in between the two operations.
>
> 3. The accessor getMachine(key) method effectively does a double check to
> decide whether it needs to create the entry. The first check is against
> null
> and the second is an implicit check during the call to putIfAbsent().
>
> 4. My first attempt didn't wrap the map with an atomic, instead the map was
> declared directly as a field. I rejected this because the set method
> involved calling clear() then putAll() which can cause a race condition if
> another thread gets in between the two operations. Was I right to reject
> this approach?
>
>
> public final class StandardSettings {
>        private final AtomicBoolean statusMessage;
>
>        private final AtomicReference<ConcurrentHashMap<String,
> MachineConfig>> machines;
>
>        public StandardSettings() {
>                this.statusMessage = new AtomicBoolean(true);
>                this.machines = new
> AtomicReference<ConcurrentHashMap<String,
> MachineConfig>>(
>                                new ConcurrentHashMap<String,
> MachineConfig>());
>        }
>
> // getter and setter for a boolean property
>
>        public Boolean getStatusMessage() {
>                return statusMessage.get();
>        }
>
>        public void setStatusMessage(Boolean statusMessage) {
>                this.statusMessage.set(statusMessage);
>        }
>
> // getter and setter for map property
>
>        public ConcurrentHashMap<String, MachineConfig> getMachines() {
>                return new ConcurrentHashMap<String,
> MachineConfig>(machines.get());
>        }
>
>        public void setMachines(ConcurrentHashMap<String, MachineConfig>
> machines) {
>                this.machines.set(machines);
>        }
>
> // access to the named map entry that creates that entry if it doesn't
> exist
>
>        public MachineConfig getMachine(String name) {
>                MachineConfig config = machines.get().get(name);
>
>                if (config == null) {
>                        MachineConfig newValue = new MachineConfig(name);
>                        // another thread may have just added after our null
> check
>                        MachineConfig oldValue =
> machines.get().putIfAbsent(name, newValue);
>                        config = oldValue != null ? oldValue : newValue;
>
>                }
>
>                return config;
>        }
>
> }
>
> Am I on the right track for making this class thread safe and if not how
> should I be approaching the problem?
>
> Many thanks
> - Ashley Williams
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
> __________ Information from ESET NOD32 Antivirus, version of virus
> signature
> database 4179 (20090622) __________
>
> The message was checked by ESET NOD32 Antivirus.
>
> http://www.eset.com
>
>
>
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20090623/608c5615/attachment-0001.html>


More information about the Concurrency-interest mailing list