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

Fix refcounting of libsingular rings #13450

Closed
simon-king-jena opened this issue Sep 12, 2012 · 6 comments
Closed

Fix refcounting of libsingular rings #13450

simon-king-jena opened this issue Sep 12, 2012 · 6 comments

Comments

@simon-king-jena
Copy link
Member

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:

  1. In sage/libs/singular/ring.pyx, copy the strings manually, such as
   for 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

provided that the parameter perturb is at least 7.
2. 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
  1. 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.

Depends on #11521

Upstream: None of the above - read trac for reasoning.

CC: @nbruin @malb

Component: commutative algebra

Reviewer: Nils Bruin

Issue created by migration from https://trac.sagemath.org/ticket/13450

@nbruin
Copy link
Contributor

nbruin commented Sep 12, 2012

comment:1

duplicate of #13447?

@simon-king-jena
Copy link
Member Author

comment:2

Replying to @nbruin:

duplicate of #13447?

Sure.

@simon-king-jena
Copy link
Member Author

Reviewer: Nils Bruin

@simon-king-jena
Copy link
Member Author

comment:3

I guess that makes it a positive review with you as reviewer, with the suggested resolution "duplicate of #13447 ".

@simon-king-jena
Copy link
Member Author

comment:5

Jeroen, why "sage-pending"? It is clear that this ticket is a duplicate of #13447.

@jdemeyer
Copy link

comment:6

Replying to @simon-king-jena:

It is clear that this ticket is a duplicate of #13447.

Then the milestone should be set to sage-duplicate/invalid/wontfix.

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

4 participants