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

Finalisers on embed fields don't get run #1551

Closed
dipinhora opened this issue Feb 3, 2017 · 1 comment
Closed

Finalisers on embed fields don't get run #1551

dipinhora opened this issue Feb 3, 2017 · 1 comment
Assignees
Labels
triggers release Major issue that when fixed, results in an "emergency" release

Comments

@dipinhora
Copy link
Contributor

Finalisers do not get run for embeds.

@sylvanc pointed out that this might be an issue. I verified it by extending the minimal-cases/finalisers code to confirm.

The following code:

primitive PrimitiveInitFinal
  fun _init() =>
    @printf[I32]("primitive init\n".cstring())

  fun _final() =>
    @printf[I32]("primitive final\n".cstring())

class ClassFinal
  embed f: EmbedFinal = EmbedFinal

  fun _final() =>
    @printf[I32]("class final\n".cstring())

class EmbedFinal
  fun _final() =>
    @printf[I32]("embed final\n".cstring())

actor Main
  new create(env: Env) =>
    ClassFinal

  fun _final() =>
    @printf[I32]("actor final\n".cstring())

Produces the following output when run:

$ ./finalisers 
primitive init
actor final
class final
primitive final

If the embed in ClassFinal is changed to a var or let the following output is produced:

$ ./finalisers 
primitive init
actor final
class final
embed final
primitive final
@Praetonus
Copy link
Member

I think the most straightforward way to fix this is to add calls to embedded fields finalisers in the enclosing objects' finalisers. That would be in the genfun_fun function in genfun.c, where a special case should be added if the function being generated is a finaliser (i.e. if its name is _final).

@Praetonus Praetonus added bug: 3 - ready for work triggers release Major issue that when fixed, results in an "emergency" release labels Feb 3, 2017
@Praetonus Praetonus self-assigned this Feb 14, 2017
Praetonus pushed a commit to Praetonus/ponyc that referenced this issue Feb 14, 2017
Previously, finalisers of embedded fields would never be called due to
embedded fields being transparent to the runtime. This change adds
calls to the adequate finalisers in parent objects' finalisers when
said parents contain embedded fields.

Closes ponylang#1551.
Praetonus pushed a commit to Praetonus/ponyc that referenced this issue Feb 14, 2017
Previously, finalisers of embedded fields would never be called due to
embedded fields being transparent to the runtime. This change adds
calls to the adequate finalisers in parent objects' finalisers when
said parents contain embedded fields.

Closes ponylang#1551.
sylvanc pushed a commit that referenced this issue Feb 15, 2017
Previously, finalisers of embedded fields would never be called due to
embedded fields being transparent to the runtime. This change adds
calls to the adequate finalisers in parent objects' finalisers when
said parents contain embedded fields.

Closes #1551.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triggers release Major issue that when fixed, results in an "emergency" release
Projects
None yet
Development

No branches or pull requests

2 participants