CODEWITHSUNDEEP
javamultithreadingthread-safety
blah_crusader
blah_crusader

Reputation: 434

Java: Fill a matrix multi-threaded, currently no thread-safe

I'm supernew to multi-threading, but I think I got the overall idea more or less. I'm trying to fill a matrix multi-threaded, but my code is clearly not thread-safe, I have duplicate columns in my matrix, which is not the case when the matrix is filled regularly. Below is an example block of code. Note that the reader is a Scanner object and someOperationOnText(someText) returns an int[100] object.

    int[][] mat = new int[100][100];
    ExecutorService threadPool = Executors.newFixedThreadPool(8);

    for (int i = 0; i < 100; i++) {
        Set<Integer>someText = new HashSet<>(reader.next());
        int lineIndex = i;
        threadPool.submit(() -> mat[lineIndex] = someOperationOnText(someText);
   }

Do you see any reason why this is not thread-safe? I can't seem to get my head around it, since the reading is done outside the thread-pool, I didn't think it would be at risk.

Thanks a lot for any debugging tips! Grts

Upvotes: 0

Views: 190

Answers (2)

Stephen C
Stephen C

Reputation: 719307

There is a happens-before between the submit calls and the execution of the lambda by a thread in the executor's thread pool. (See javadoc: "Memory consistency effects"). So that means the lambda will see the correct values for someText, mat and lineIndex.

The only thing that is not thread-safe about this is the (implied) code that uses the values in mat in the main thread. Calling shutdown() on the executor should be sufficient ... though the javadocs don't talk about the memory consistency effects of shutdown() and awaitTermination().

(By my reading of the code for ThreadPoolExecutor, the awaitTermination() method provides a happens-before between the pool threads (after completion of all tasks) and the method's return. The happens-before is due the use of the executor's main lock to synchronize the pool shutdown. It is hard to see how they could implement shutdown correctly without this (or equivalent), so it is more than an "implementation artefact" ... IMO.)

Upvotes: 1

Sammers
Sammers

Reputation: 1108

Well, accessing elements in the matrix is a very fast operation, a computing is not.

I think making access to a matrix synchronized while computing can be concurrent is a right approach for you.

    int[][] mat = new int[100][100];
    Object lock = new Object();
    ExecutorService threadPool = Executors.newFixedThreadPool(8);

    for (int i = 0; i < 100; i++) {
        Set<Integer> someText = new HashSet<>(reader.next());
        int lineIndex = i;
        threadPool.submit(() -> {
            int result = someOperationOnText(someText);
            synchronized (lock) {
                mat[lineIndex] = result;
            }
        });
    }

Upvotes: 0

Related Questions