[concurrency-interest] concurrency newbie question

Chris Kessel/Lou Doherty chriskessel at verizon.net
Mon Jun 22 23:26:45 EDT 2009


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





More information about the Concurrency-interest mailing list