Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple race conditions #66

Closed
bashdi opened this issue Nov 6, 2024 · 2 comments · Fixed by #68
Closed

Multiple race conditions #66

bashdi opened this issue Nov 6, 2024 · 2 comments · Fixed by #68
Assignees
Labels
bug Something isn't working

Comments

@bashdi
Copy link

bashdi commented Nov 6, 2024

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 SanjulaGanepola self-assigned this Nov 22, 2024
@SanjulaGanepola SanjulaGanepola added the bug Something isn't working label Nov 22, 2024
@SanjulaGanepola
Copy link
Member

@bashdi Thank you for sharing this issue. To address the problems with the Query and SqlJob classes, I have started this PR: #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?

@SanjulaGanepola
Copy link
Member

@bashdi The changes in #68 to address this issue with be released in v0.1.0. If you encounter any more issues related to this, please re-open this one or create a new issue.

@SanjulaGanepola SanjulaGanepola linked a pull request Dec 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants