Skip to content

GC now collect cycles#2404

Merged
privat merged 2 commits intonitlang:masterfrom
BlackMinou:gc_modifs
Apr 5, 2017
Merged

GC now collect cycles#2404
privat merged 2 commits intonitlang:masterfrom
BlackMinou:gc_modifs

Conversation

@BlackMinou
Copy link
Copy Markdown
Contributor

@BlackMinou BlackMinou commented Apr 3, 2017

This fixes an issue with the actor model where the GC is unable to collect cycles associated with actors objects and printing an annoying warning

Theses modifs should fix the tests in #2401

The downside is that the finalization is no longer in topological order

Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Comment thread lib/core/gc.nit
#
# The object are finialized in a topological order, it is safe for this method
# to use attributes of this instances.
# The object are not finialized in a topological order, it is safe for this method
Copy link
Copy Markdown
Contributor

@jcbrinfo jcbrinfo Apr 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence should end at the first comma IMHO. Example:

The objects are not finialized in a topological order. It is safe for this method […]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok ...

Copy link
Copy Markdown
Contributor

@xymus xymus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was overdue, thanks!

privat added a commit that referenced this pull request Apr 5, 2017
This fixes an issue with the actor model where the GC is unable to collect cycles associated with actors objects and printing an annoying warning

Theses modifs should fix the tests in #2401

The downside is that the finalization is no longer in topological order

Pull-Request: #2404
@privat privat merged commit 81cdd7c into nitlang:master Apr 5, 2017
@privat privat removed the need_review label Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants