Opened 7 years ago
Closed 7 years ago
#13450 closed defect (duplicate)
Fix refcounting of libsingular rings
Reported by: | SimonKing | Owned by: | malb |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | commutative algebra | Keywords: | |
Cc: | nbruin, malb | Merged in: | |
Authors: | Reviewers: | Nils Bruin | |
Report Upstream: | None of the above - read trac for reasoning. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #11521 | Stopgaps: |
Description
While working on #715/#11521, a sporadic error has occurred:
On bsd.math (hence, OS X on Intel chips), sage -t sage/misc/cachefunc.pyx
fails with signal 11. Apparently it is fine on different machines, even though with additional patches (such as #13370 or #12876) there are signal 11 in other tests and on other machines as well.
The segfault disappears when running the tests in verbose mode. It also disappears if the tests are run under gdb (but then a different error occurs, that is unrelated with the problem we are dealing with here).
Nils (see #715) was able to track the problem down to libsingular. He found (correct me if I misunderstood):
A segfault occurs in MPolynomialRing_libsingular.__init__
in the line:
self._ring = singular_ring_new(base_ring, n, self._names, order)
Digging deeper, he came to sage/libs/singular/ring.pyx
in the lines:
_names = <char**>omAlloc0(sizeof(char*)*(len(names))) for i from 0 <= i < n: _name = names[i] _names[i] = omStrDup(_name)
Strange enough, the segfault occurs in omStrDup
.
He found several ways to work around:
- In
sage/libs/singular/ring.pyx
, copy the strings manually, such asprovided that the parameterfor i from 0 <= i < n: _name = names[i] j = 0 while <bint> _name[j]: j+=1 j+=1 #increment to include the 0 copiedname = <char*>omAlloc(sizeof(char)*(j+perturb)) for 0 <= offset < j: copiedname[offset] = _name[offset] _names[i] = copiedname
perturb
is at least 7. - Modify manual refcounting in
sage/libs/singular/ring.pyx
to prevent deallocation:wrapped_ring = wrap_ring(_ring) if wrapped_ring in ring_refcount_dict: raise ValueError('newly created ring already in dictionary??') - ring_refcount_dict[wrapped_ring] = 1 + ring_refcount_dict[wrapped_ring] = 2 return _ring
- Use a strong cache for multivariate polynomial rings.
Options 2. and 3. would not be nice: The aim of #715, #11521, #12215 and #12313 was to make caches for parents weak in order to prevent memory leaks.
Since the problem disappears when deallocation of libsingular rings is prevented, it seems that the bug is in the deallocation of libsingular rings. Note that the problem is not fixed by #13145.
I don't know if it is an upstream bug or a bug in the Sage library. Hence, no idea what to report upstream.
Change History (7)
comment:1 follow-up: ↓ 2 Changed 7 years ago by
comment:2 in reply to: ↑ 1 Changed 7 years ago by
- Status changed from new to needs_review
comment:3 Changed 7 years ago by
- Reviewers set to Nils Bruin
- Status changed from needs_review to positive_review
I guess that makes it a positive review with you as reviewer, with the suggested resolution "duplicate of #13447 ".
comment:4 Changed 7 years ago by
- Milestone changed from sage-5.4 to sage-pending
comment:5 follow-up: ↓ 6 Changed 7 years ago by
Jeroen, why "sage-pending"? It is clear that this ticket is a duplicate of #13447.
comment:6 in reply to: ↑ 5 Changed 7 years ago by
- Milestone changed from sage-pending to sage-duplicate/invalid/wontfix
comment:7 Changed 7 years ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
duplicate of #13447?