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.

Sign up for GitHub

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

Jump to bottom

LibGit2 should not extend Base.finalize #17414

Closed
yuyichao opened this issue Jul 14, 2016 · 2 comments
Closed

LibGit2 should not extend Base.finalize #17414

yuyichao opened this issue Jul 14, 2016 · 2 comments
Labels
bug Indicates an unexpected problem or unintended behavior libgit2 The libgit2 library or the LibGit2 stdlib module

Comments

@yuyichao
Copy link
Contributor

Base.finalize has well define meaning and the overload doesn't do that.

Ref #17409 (comment)
@tkelman tkelman added the libgit2 The libgit2 library or the LibGit2 stdlib module label Jul 14, 2016
@JeffBezanson JeffBezanson added the bug Indicates an unexpected problem or unintended behavior label Jul 22, 2016
@JeffBezanson
Copy link
Member

More importantly, if the intent was to add finalizers for these objects, then it's not working and the objects are never getting finalized.

@JeffBezanson
Copy link
Member

Ah, I see the code uses with. If all objects are guaranteed to be freed that way, then these can just become methods of close instead.

simonbyrne added a commit that referenced this issue Dec 20, 2016
@simonbyrne
Overload Base.close instead of Base.finalize for LibGit2
81cff89 
Fixes #17414.
@simonbyrne simonbyrne mentioned this issue Dec 20, 2016
Merged
simonbyrne added a commit that referenced this issue Dec 30, 2016
@simonbyrne
Overload Base.close instead of Base.finalize for LibGit2
13933da 
Fixes #17414.
simonbyrne added a commit that referenced this issue Jan 4, 2017
@simonbyrne
Overload Base.close instead of Base.finalize for LibGit2
a7ff87c 
Fixes #17414.
simonbyrne added a commit that referenced this issue Jan 7, 2017
@simonbyrne
Overload Base.close instead of Base.finalize for LibGit2
71eca92 
Fixes #17414.
tkelman pushed a commit that referenced this issue Jan 9, 2017
@simonbyrne @tkelman
Attach finalizers to LibGit2 methods (#19660)
db4aaa3 
* Overload Base.close instead of Base.finalize for LibGit2

Fixes #17414.

* fix test/pkg.jl

* define finalizers

* try qualifying method

* keep reference to parent repo

* fix conflict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@yuyichao @JeffBezanson @tkelman