CODEWITHSUNDEEP
javamultithreadingconcurrency
Zack
Zack

Reputation: 871

Sharing Variables with Concurrency

I am trying to a write a Java program that has multiple threads. I have two variables that are created on startup. One is an AtomicBoolean that tells the whole program when to shutdown (i.e close the clients). Another variable is a ConcurrentHashMap that contains a set of commands that are loaded in at startup. There are threads that contain a client runnable that run when a client has been accepted (one thread per client). There is also a thread that updates the commands to the HashMap by listening for external changes (new commands from a directory, etc.)

Here is the code:

class Program {
    /* ... other vars like ServerSocket, etc. */
    private final ConcurrentHashMap<String, String> mCommands;
    private final AtomicBoolean mIsListening;

    public static void main(String[] args) {
        Program prog = new Program();
        prog.loadCommands();
        prog.listen();
    }

    public Program() {
        mCommands = new ConcurrentHashMap<>();
        mIsListening = new AtomicBoolean(true);
        /* other initializations */
    }

    public void loadCommands() {
         /* Loads commands generated from a directory; updates mCommands, then... */
         new Thread(new CommandListenerRunnable(mCommands)).start();
    }

    public void listen() {
        /* accepting a new client from server socket */
        while (mIsListening.get()) {
            Socket client = ss.accept();
            new Thread(new ClientRunnable(client, mCommands, mIsListening)).start();
        }
    }
}

As you can see, with every child that I am creating, I am passing these variables as arguments. All in all, is this the proper way to distribute these variables? Any thoughts would be greatly appreciated. Thanks in advance!

Upvotes: 1

Views: 125

Answers (2)

ddarellis
ddarellis

Reputation: 3960

I think the design is good because each thread should have a copy of the variables it needs.

You said:

For instance, I can use map.putIfAbsent() to make the operation be performed atomically

but this does not make your map thread safe because someone could call something else on it.

Upvotes: 3

Patrick Parker
Patrick Parker

Reputation: 4959

Well, it's not foolproof. Read all the caveats in the documentation of ConcurrentHashMap such as--

"the results of aggregate status methods including size, isEmpty, and containsValue are typically useful only when a map is not undergoing concurrent updates in other threads."

If those limitations are acceptable, then what you have is fine. Otherwise you will need your own locking scheme to govern access.

It could be something as simple as synchronized(sharedLockingObject), or something as complex as a ReentrantReadWriteLock.

Upvotes: 2

Related Questions