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

Closing BGZI gives a 'munmap_chunk(): invalid pointer' #86

Open
xbello opened this issue Feb 12, 2024 · 3 comments
Open

Closing BGZI gives a 'munmap_chunk(): invalid pointer' #86

xbello opened this issue Feb 12, 2024 · 3 comments

Comments

@xbello
Copy link
Contributor

xbello commented Feb 12, 2024

Using Nim 2.0.2, hts-nim 0.3.25.

(Also, depending of the source, the error is SIGSEGV: Illegal storage access. (Attempt to read from nil?)).

The minimal example I can do to make it fail is this:

var b: BGZI
doAssert(variants.open("tests/test_files/sample.tsv.gz")
b.close()

When compiled with ARC/ORC, it runs but fails with the error menctioned in the title 'munmap_chunk(): invalid pointer'. If compiled with the flag -mm:refc everything goes smoothly.

I think I've pinpointed the problem to hts_idx_set_meta

return int(hts_idx_set_meta(idx, uint32(len(csi.meta)), csi.meta[0].addr, do_copy))

because if you return early in with return 0 just before the true return, the error goes away. It seems that there were problems here before (samtools/htslib#936).

Contents of "sample.tsv.gz":

chr1	10001	T	A
chr1	10002	A	C
chr12	10003	A	G

With a "sample.tsv.gz.csi" file besides it.

brentp added a commit that referenced this issue Feb 14, 2024
but still lots of problems with orc that need to be resolved
brentp added a commit that referenced this issue Feb 14, 2024
but still lots of problems with orc that need to be resolved
@brentp
Copy link
Owner

brentp commented Feb 14, 2024

Hi Xabier, thanks for reporting. I pushed a fix for this.
There are still many things that need to change throughout hts-nim to be fully compatible with ORC/ARC. I'll keep checking those off as I have time, but PRs are also welcome.
Please let me know if you hit other issues.

@xbello
Copy link
Contributor Author

xbello commented Feb 14, 2024

I would love to help you more on this, I spend a lot of time with this issue trying to fix it and make a PR. But I couldn't understand what does the proc idx_set_meta with all that pointer code. Best I could do was to isolate the failure and create the issue.

Right now I'm overriding the BGZI.close with my own procedure that just skips the CSI sync step, which is less than ideal, but at least my code compiles with ORC and runs correctly (it only reads the BGZI files).

@brentp
Copy link
Owner

brentp commented Feb 14, 2024

Thanks for the test files and diagnosing the exact location of the problem!
I can't remember how idx_set_meta works either. 😢

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

No branches or pull requests

2 participants