Opened 7 years ago
Last modified 7 years ago
#18385 needs_info defect
deallocate pari when sage quits
Reported by: | Nathann Cohen | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.7 |
Component: | memleak | Keywords: | |
Cc: | Leif Leonhardy, Jean-Pierre Flori | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | public/18385 (Commits, GitHub, GitLab) | Commit: | 9bb69f848059d05ecb6566fa99d6e5d0624b6fa1 |
Dependencies: | Stopgaps: |
Description (last modified by )
As global module variables point toward the (unique) PariInstance
instance built by Sage, and because those are not automatically cleaned, this instance is never deleted. Thus, we do it explicitly in quit_sage
, as it was done before #13741.
Nathann
[1] https://groups.google.com/d/topic/sage-devel/E-U4otPW9_0/discussion
Change History (10)
comment:1 Changed 7 years ago by
Branch: | → public/18385 |
---|---|
Status: | new → needs_review |
comment:2 Changed 7 years ago by
Commit: | → 9bb69f848059d05ecb6566fa99d6e5d0624b6fa1 |
---|
comment:3 follow-up: 4 Changed 7 years ago by
Status: | needs_review → needs_info |
---|
I don't like this. How can you be sure that no PARI code will be executed after deallocating the PARI stack?
comment:4 Changed 7 years ago by
I don't like this. How can you be sure that no PARI code will be executed after deallocating the PARI stack?
I am not, and you are right this is probably not a good idea after all. Do you know how we could do to have this be deallocated when it should?
comment:5 Changed 7 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 7 years ago by
Of course it's not nice, but the only alternatives that come to my mind are:
- Add even more Valgrind suppressions. (IMHO odd.)
- Remove the global variable which is bad anyway. (Implies a performance penalty.)
- Convince Cython to call the deallocator. (I have no idea how;
del
inquit_sage()
doesn't work.)
comment:8 Changed 7 years ago by
Cc: | Jean-Pierre Flori added |
---|
comment:9 follow-up: 10 Changed 7 years ago by
I am against solving this problem "by proxy". The problem isn't that pari
is never deallocated, the problem is apparently that Cython global variables are never deallocated. Even if you fix pari, then there are still a lot of other global variables with the same problem.
comment:10 Changed 7 years ago by
Replying to jdemeyer:
The problem isn't that
pari
is never deallocated, the problem is apparently that Cython global variables are never deallocated.
A little googling gives me the impression that this is a wide-spread problem with python (extension) modules: there is no way to "unload" a module (which would be the time to deallocate global variables), and it seems it doesn't exist because people haven't been able to solve the subtle problems that arise in deciding when and in what order unloading should happen (especially extension modules seem to be complicated this way)
Indeed, without extra information, the fact that arbitrary code could be executed upon deallocation means there is no sure-fire way to decide a correct way to order module deallocation. It wouldn't be hard to write 2 modules A and B such that the deallocation code of one requires the other module to be functional. One would require a rather careful protocol specification to allow module deactivation.
Without extra information, you cannot start deallocating global variables unless you know the module has been "deactivated". So I think Cython in general will not be solving this problem any time soon.
Branch pushed to git repo; I updated commit sha1. New commits:
trac #18385: deallocate pari when sage quits