Mapepire-IBMi / mapepire-java

Mapepire Java Client SDK
https://central.sonatype.com/artifact/io.github.mapepire-ibmi/mapepire-sdk
Apache License 2.0
5 stars 2 forks source link

Multiple race conditions #66

Open bashdi opened 2 weeks ago

bashdi commented 2 weeks ago

Looking at the code, I found a few problems with this Lib and threadsafty.

possible race conditions:

Class Query

private static List<Query> globalQueryList = new ArrayList<>();

    public Query(SqlJob job, String query, QueryOptions opts) {
        this.job = job;
        this.sql = query;
        this.isPrepared = opts.getParameters() != null;
        this.parameters = opts.getParameters();
        this.isCLCommand = opts.getIsClCommand();
        this.isTerseResults = opts.getIsTerseResults();

        Query.globalQueryList.add(this);
    }

Creating multiple Query-Objects at the same time -> race condition

Class SqlJob

private static int uniqueIdCounter;
public static String getNewUniqueId() {
        return SqlJob.getNewUniqueId("id");
    }

Calling the method getNewUniqueId which will be called on every interaction with the server -> race conditon

 private JobStatus status = JobStatus.NotStarted;

If multple threads work with the same SQLJob-Object at the same time, which the Pool-Class allows, changing the status leads to -> race condition.

Not sure, if its a good idea to be able to ask the Pool for a SqlJob-Object without ensuring exclusivity through marking or removing it from the pool.

SanjulaGanepola commented 3 days ago

@bashdi Thank you for sharing this issue. To address the problems with the Query and SqlJob classes, I have started this PR: https://github.com/Mapepire-IBMi/mapepire-java/pull/68

If multple threads work with the same SQLJob-Object at the same time, which the Pool-Class allows, changing the status leads to -> race condition.

Not sure, if its a good idea to be able to ask the Pool for a SqlJob-Object without ensuring exclusivity through marking or removing it from the pool.

As for this issue, when getJob is called on a Pool, it will get a job as fast as possible. This means it will either be a ready job or the job with the least requests on the queue. Thus, if multiple threads are using the same SqlJob from a Pool, each request should just be queued and thus I expect that the status changing should not have a race condition. Could you share an example snippet to reproduce the issue?