Modify outer object from inside a thread

Question

public void fooAndBar() {
    HashMap<Foo, Bar> fooBarMap = new HashMap<>();
    CompletionService completionService = new ExecutorCompletionService(exec);

    for(int i=0; i<10; i++) {
        completionService.submit(new Callable() {
            @Override
            public Void call() throws Exception {
                fooBarMap.put(new Foo(i), new Bar(i));
                return null;
            }
        });
    }
}

• Is it safe to modify the HashMap inside the Callable?

• Should the hashmap be final (or maybe volatile) and if so, why?

• Should I use a structure other than HashMap, something like ConcurrentHashMap or SynchronizedMap and why?

I'm trying to grasp java concepts so please bear with me

Answer 1

Is it safe to modify the HashMap inside the Callable?

No. If you are using a threadpool I assume you are planning to have more of those callables running in parallel. Any time an object with mutable state is accessed from more than one thread, that's thread-unsafe. If you write to a thread-unsafe hashmap from two threads simultaneously, its internal structure will be corrupted. If you read from a thread-unsafe hashmap while another thread is writing to it simultaneously, your reading thread will read garbage. This is a very well known and extensively studied situation known as a Race Condition, a description of which would be totally beyond the scope of this answer. For more information, read about Race Condition on Wikipedia or on another question answered back in 2008: Stackoverflow - What is a Race Condition.

Should the hashmap be final (or maybe volatile) and if so, why?

For your purposes it does not need to be final, but it is always a good practice to make final anything that can be made final.

It does not need to be volatile because:

• if you were to make it volatile, you would be making the reference to it volatile, but the reference never changes, it is its contents that change, and volatile has nothing to do with those.

• the threadpool makes sure that call() will be executed after fooBarMap = new HashMap<>(). (If you are wondering why such a thing could ever be a concern, google for "memory boundary".)

Should I use a structure other than HashMap, something like ConcurrentHashMap or SynchronizedMap and why?

Definitely. Because, as I wrote earlier, any time an object with mutable state is accessed from more than one thread, that's thread-unsafe. And ConcurrentHashMap, SynchronizedMap, synchronize, etc. exist precisely for taking care of thread-unsafe situations.