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

NTL "context"s can be restored at the wrong time, leading to randomly-wrong answers #5340

Closed
sagetrac-cwitty mannequin opened this issue Feb 22, 2009 · 4 comments
Closed

Comments

@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Feb 22, 2009

I should have a fix for this very soon.

Component: number theory

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

@sagetrac-cwitty sagetrac-cwitty mannequin added this to the sage-3.4 milestone Feb 22, 2009
@sagetrac-cwitty sagetrac-cwitty mannequin self-assigned this Feb 22, 2009
@sagetrac-cwitty
Copy link
Mannequin Author

sagetrac-cwitty mannequin commented Feb 22, 2009

comment:2

The problem is that __dealloc__ can happen at "random" times (whenever the garbage collector happens to trigger), so it must not have global side-effects.

To the reviewer: Note that the NTL documentation explicitly says you don't need to have the correct context when you destroy an object:

Note, however, that if a GF2E object is created under one modulus 
and then used in any way (except destroyed) under another, 
program behavior is not predictable.

Essentially identical language occurs in the documentation for lzz_pE, lzz_p, ZZ_pE, and ZZ_p.

I fixed 9 potential instances of the problem, but only added a doctest for one of them; you'll understand why when you see how hard it is to doctest.

All doctests pass.

This is based on sage-3.3 + ReST patches, but I think it would probably apply without the ReST patches just fine.

@sagetrac-cwitty
Copy link
Mannequin Author

sagetrac-cwitty mannequin commented Feb 22, 2009

Attachment: trac5340-ntl-contexts-and-dealloc.patch.gz

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Feb 24, 2009

comment:3

Positive review. I did valgrind all of the Sage 3.3 doctests + this patch (while testing gsw's libSingular work) and no issue popped up. It also passes doctests on top of the ReST patches.

Cheers,

Michael

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Feb 24, 2009

comment:4

Merged in Sage 3.4.alpha0.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Feb 24, 2009
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

0 participants