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

Non threadsafe code in QueryProviderService #30

Closed
pardahlman opened this issue Jan 2, 2018 · 0 comments
Closed

Non threadsafe code in QueryProviderService #30

pardahlman opened this issue Jan 2, 2018 · 0 comments

Comments

@pardahlman
Copy link
Contributor

pardahlman commented Jan 2, 2018

The QueryProviderService resolves a list of IQueryProvider based on the supplied command. Each provider kicks of an task that performs the query and updates the same IList<IQueryResult>. Here's a code snippet

for (var i = target.Count - 1; i >= 0; i--)
{
    var current = target[i];
    if (!result.Contains(current))
    {
        target.Remove(target[i]);
    }
}

If two threads simultaneous enters this for loop they would both get the same initial value for i, which is a problem in the following scenario:

  • One of the threads execute the entire code block and removes the element before the second thread tries to access target[i], and an index out of bounds exception will be thrown
  • The threads execute in parallel but in the remove statement they both access target[i] and one of them is going to succeed and the other one would get an index out of bounds

The reason this is not a problem today is that Jarvis only has three query providers, and none of them are overlapping.

I think it would make sense to execute the query providers in parallel, but edit the list in once all providers have returned their results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants