IBM / JTOpen

IBM Toolbox for Java, an IBM i communications library
https://ibm.github.io/JTOpen/
Other
60 stars 28 forks source link

Fix race condition initializing attributes data structure #204

Closed dwickern closed 1 week ago

dwickern commented 2 weeks ago

I occasionally get an error when calling SpooledFile.getIntegerAttribute(PrintObject.ATTR_PAGES):

com.ibm.as400.access.ExtendedIllegalArgumentException: ATTR_PAGES: Parameter value is not valid.
    at com.ibm.as400.access.PrintObjectImplRemote.getIntegerAttribute(PrintObjectImplRemote.java:133)
    at com.ibm.as400.access.PrintObject.getIntegerAttribute(PrintObject.java:844)

The code in question is multithreaded, but the AS400 are not shared across threads.

Reproduction

Fill in the connection info and use the attributes of any spooled file on the system.

import com.ibm.as400.access.AS400;
import com.ibm.as400.access.PrintObject;
import com.ibm.as400.access.SpooledFile;

import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executors;

public class Reproduction {
    public static void main(String[] args) throws Exception {
        // connection info
        var system = "";
        var userId = "";
        var password = "";

        // spooled file info
        var name = "";
        var number = 0;
        var jobName = "";
        var jobUser = "";
        var jobNumber = "";

        var executor = Executors.newCachedThreadPool();
        try {
            var threadCount = 5;
            var latch = new CountDownLatch(threadCount);

            Callable<String> task = () -> {
                // one AS400 instance per thread
                try (var as400 = new AS400(system, userId, password)) {
                    as400.connectService(AS400.PRINT);
                    var spooledFile = new SpooledFile(as400, name, number, jobName, jobUser, jobNumber);

                    // wait for all threads to maximize the chance of race condition
                    latch.countDown();

                    // use the last attribute value to maximize the chance of race condition
                    return spooledFile.getStringAttribute(PrintObject.ATTR_USRDEFOPT);
                }
            };
            var results = executor.invokeAll(Collections.nCopies(threadCount, task));
            for (var result : results) {
                System.out.println("ATTR_USRDEFOPT = " + result.get());
            }
        } finally {
            executor.shutdown();
        }
    }
}

Solution

The spooled file attributes data structure is shared across threads: https://github.com/IBM/JTOpen/blob/5702fe2878e89b0f4b9940763e2282d16753f045/src/main/java/com/ibm/as400/access/SpooledFileImplRemote.java#L36-L37

The first SpooledFile to access the attributes will initialize the data structure: https://github.com/IBM/JTOpen/blob/5702fe2878e89b0f4b9940763e2282d16753f045/src/main/java/com/ibm/as400/access/SpooledFileImplRemote.java#L461-L467

The problem is that fAttrIDsToRtvBuilt_ is set true before being fully initialized, so the next thread can see a partially initialized attributes: https://github.com/IBM/JTOpen/blob/5702fe2878e89b0f4b9940763e2282d16753f045/src/main/java/com/ibm/as400/access/SpooledFileImplRemote.java#L137-L144

The solution in this PR is to set fAttrIDsToRtvBuilt_ = true after the attributes are fully initialized.

The same pattern is used in some other places besides SpooledFile, so I fixed those too.