Opened 13 years ago
Closed 13 years ago
#5839 closed defect (fixed)
[with patch, positive review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash
Reported by: | cwitty | Owned by: | cwitty |
---|---|---|---|
Priority: | critical | Milestone: | sage-4.0 |
Component: | commutative algebra | Keywords: | |
Cc: | malb | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
In __dealloc__
, if currRing is NULL on entry, then currRing will be the ring we just deleted on exit. The patch fixes this bug, so that currRing never points to freed memory.
It took me quite a while to come up with a small reproducible test case for the problem; here it is. (This test case is also in the patch, as a doctest.)
import gc from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular R1 = MPolynomialRing_libsingular(GF(5), 2, ('x', 'y'), TermOrder('degrevlex', 2)) R2 = MPolynomialRing_libsingular(GF(11), 2, ('x', 'y'), TermOrder('degrevlex', 2)) R3 = MPolynomialRing_libsingular(GF(13), 2, ('x', 'y'), TermOrder('degrevlex', 2)) gc.collect() foo = R1.gen(0) del foo del R1 gc.collect() del R2 gc.collect() del R3 gc.collect()
Attachments (1)
Change History (10)
Changed 13 years ago by
comment:1 Changed 13 years ago by
- Summary changed from [with patch, needs review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash to [with patch,positive review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash
comment:2 Changed 13 years ago by
- Milestone changed from sage-3.4.1 to sage-3.4.2
comment:3 Changed 13 years ago by
- Resolution set to fixed
- Status changed from new to closed
- Summary changed from [with patch,positive review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash to [with patch, positive review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash
Merged in Sage 3.4.2.alpha0.
Cheers,
Michael
comment:4 Changed 13 years ago by
- Resolution fixed deleted
- Status changed from closed to reopened
Mmmh, I seem to be seeing random doctest failure with 3.4.1 + only this patch merged, so I am reopening this ticket for now. Can someone do extensive testing on sage.math to see if they can reproduce it?
Cheers,
Michael
comment:5 Changed 13 years ago by
Ok, given that this causes issues for me I am changing it to 'needs work' for now.
Cheers,
Michael
comment:6 Changed 13 years ago by
- Summary changed from [with patch, positive review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash to [with patch, needs work] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash
comment:7 Changed 13 years ago by
What sort of errors? (Crashes? Wrong answers? Any particular files?)
comment:8 Changed 13 years ago by
- Summary changed from [with patch, needs work] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash to [with patch, positive review] MPolynomialRing_libsingular __dealloc__ is buggy, can lead to crash
I have come to believe that the issues I saw were due to other issues, so I am reinstating the positive review. But I will do some more extensive testing before merging this. Sorry for the noise.
Cheers,
Michael
comment:9 Changed 13 years ago by
- Milestone changed from sage-4.0.1 to sage-4.0
- Resolution set to fixed
- Status changed from reopened to closed
Merged in Sage 4.0.alpha0.
Cheers,
Michael
Doctests pass, patch reads good.