Repository navigation
PR #4539 backport to v1.0.x with Python 2.6 support - #4540
Merged
Merged
Conversation
as explained here: https://github.com/astropy/astropy/pull/520/files#diff-8ba48829876ba755e67054a1a90ec124R2367 I will add more in-line documentation of this in a subsequent commit. But basically what's going on is this: Whenever a Column object's .array object is assigned to, it actually walks that array's bases to check for the reference cycle, by seeing if one of those bases is a FITS_rec instance with the present Column in its ColDefs. If so, rather than creating a hard reference it stores a weak reference to the original FITS_rec, and the .array property returns the array via that weak reference. That way the reference cycle never gets created in the first place. On the FITS_rec end, it actually also keeps a set of weak references to all Column objects associated with that FITS_rec through its ._coldefs. When FITS_rec.__del__ is called (because its reference count has gone to zero) it deletes self._coldefs. If that ColDefs object has no references outside the FITS_rec (i.e. a user has not pointed a variable to it), it and all Columns it contains are properly deleted. This also removes the column from the FITS_rec's _col_weakrefs set. However, if there are still references to the Column (again, by a user for example) then the data for that array is copied (maybe instead we can just restore the reference to the original FITS_rec?), so the user can continue to access that data. This is allowed because by the time FITS_rec.__del__ is called its underlying memory has not been freed yet. The one exception to this was where data.deleter closed any mmap underlying the data. But I fixed the order of things so that the mmap is still available to perform that copy. This is all probably still fairly fragile, but in the vast majority of cases it fixes the chance for any reference leaks.
…ibiting the reference leak. Remove the previous note in the test for astropy#520 about the existing reference leak, since it *shouldn't* apply any longer.
… make sure that when a weakref is assigned for Column.array, that the original .array contents, if any, are dropped.
…c._coldefs) [skip ci]
…ar as the tests pass...
Member
Author
|
Just the doc build failed due to unrelated warnings (fixed in ac1cca8 which I have no backported). |
embray
added a commit
that referenced
this pull request
Jan 28, 2016
PR #4539 backport to v1.0.x with Python 2.6 support
embray
added a commit
that referenced
this pull request
Jan 28, 2016
PR #4539 backport to v1.0.x with Python 2.6 support
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This backports #4539 to the v1.0.x and v1.1.x branch (the latter can be done via cherry-pick). This required some extra work to support Python 2.6, which is still supported on those branches.