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

Do not retain unmanaged allocations in generated ctors #762

Merged
merged 1 commit into from Aug 16, 2019
Merged

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Aug 14, 2019

potentially fixes #761

@jtenner
Copy link
Contributor

jtenner commented Aug 14, 2019

I can try this later. Does the example fail in #761

@jtenner
Copy link
Contributor

jtenner commented Aug 16, 2019

I built the compiler, tested it against my example and I still get a rogue release problem. Something is wrong and I can't figure it out. Will investigate tomorrow.

@dcodeIO
Copy link
Member Author

dcodeIO commented Aug 16, 2019

The example works for me

 (func $empty/ExampleClass#constructor (; 12 ;) (type $FUNCSIG$ii) (param $0 i32) (result i32)
  local.get $0
  i32.eqz
  if
   i32.const 0
   i32.const 0
   call $~lib/rt/tlsf/__alloc
   (; retain in question was here ;)
   local.set $0
  end
  local.get $0
 )
 (func $empty/returnExample (; 13 ;) (type $FUNCSIG$i) (result i32)
  i32.const 0
  call $empty/ExampleClass#constructor
 )
 (func $start:empty (; 14 ;) (type $FUNCSIG$v)
  call $empty/returnExample
  global.set $empty/t
 )

According to rtrace t is leaking there, which is correct since it hasn't been __freed manually and is ignored by GC.

@jtenner
Copy link
Contributor

jtenner commented Aug 16, 2019

In this case, Yeah, it looks like the example is good but I've identified another unmanaged reference problem. My testing suite passes each test but crashes without explanation. Just need some time to get settled today and I promise to try and help.

@jtenner
Copy link
Contributor

jtenner commented Aug 16, 2019

Great news. I tracked down the problem. It's actually within my testing software now.

https://github.com/jtenner/as-pect/blob/master/packages/assembly/assembly/internal/report/Actual.ts#L111

This is because the reporting mechanism does not account for @unmanaged class references. Thanks for your help in this.

@dcodeIO dcodeIO merged commit 37f27b1 into master Aug 16, 2019
willemneal pushed a commit to nearprotocol/assemblyscript that referenced this pull request Aug 23, 2019
@dcodeIO dcodeIO deleted the issue-761 branch September 20, 2019 09:08
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.

[Bug] Returning an unmanaged class results in a retain
2 participants