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

Race conditions accessing CRAM's reference data through ReferenceSource #1643

Open
vruano opened this issue Jan 9, 2023 · 0 comments · May be fixed by #1645
Open

Race conditions accessing CRAM's reference data through ReferenceSource #1643

vruano opened this issue Jan 9, 2023 · 0 comments · May be fixed by #1645
Assignees
Labels

Comments

@vruano
Copy link
Contributor

vruano commented Jan 9, 2023

Description of the issue:

Problem spotted in GATK due to smelly use and no use of synchronized in CRAM ReferenceSource:
broadinstitute/gatk#8139

.../cram/.../ReferenceSource.java has a mixture of synchronized and non-synchronzed access to its cache member fields that results in a race condition when accessing it in parallel by at least one tool in GATK.

This class already contain synchronized modifier which implies that is supposed to support multi-thread access.

Your environment:

  • version of htsjdk
    Current master:9f0e80f5bf
  • version of java
    Not relevant
  • which OS
    Not relevant

Steps to reproduce

See original post. No concrete test input data provided but the problem becomes clear when inspecting the code.

Expected behaviour

Either
(a) this class is made multithread safe or
(b) it clearly stated that is not multi-thread safe and the multithread elements (e.g. synchronized modifiers) are removed.

Actual behaviour

At least there is one way to cause race conditions in multi-thread use of this class. There could be more, so code should be review for other possible issues.

vruano added a commit that referenced this issue Jan 11, 2023
…t might be trying to solve an non-existent issue.

Chaned WeakReference to SoftReference to make cache more persistent in memory.
Addresses issue #1643.
@vruano vruano added the bug label Feb 2, 2023
@vruano vruano self-assigned this Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant