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

Segfault when using a recoverd trn Array #1983

Closed
lisael opened this issue Jun 24, 2017 · 5 comments
Closed

Segfault when using a recoverd trn Array #1983

lisael opened this issue Jun 24, 2017 · 5 comments
Labels
triggers release Major issue that when fixed, results in an "emergency" release

Comments

@lisael
Copy link
Contributor

lisael commented Jun 24, 2017

this code segfaults in Bar.result

use "debug"
use "collections"

class Bar
  let _env: Env
  new create(env: Env) =>
    _env = env

  fun result(foos: Array[Foo val] val) =>
    _env.out.print("done")
    _env.out.print(try foos(0).id.string() else "error" end)

class Foo
  let id: I32
  new create(i: I32) =>
    id = i

class FooGenerator
  var foos: (Array[Foo val] val| Array[Foo val] trn) = recover trn Array[Foo val] end
  let _notify: Bar val

  new create(notify: Bar val) =>
    _notify = notify

  fun ref apply(start: I32) =>
    var i = start
    while i < (start + 10) do
      try
        (foos as Array[Foo val] trn).push(recover val Foo(i = i+1) end)
      end
    end

  fun ref stop() =>
    foos = recover val foos end
    Debug(foos.size())
    _notify.result(foos)

actor Caller
  let foogen: FooGenerator
  new create(f: FooGenerator iso) =>
    foogen = consume f

  be apply(i: I32) =>
    foogen(i)

  be stop() =>
    foogen.stop()

actor Main
  new create(env: Env) =>
    let b = recover val Bar(env) end
    let f = recover FooGenerator(b) end
    let c = Caller(consume f)
    for i in Range(0, 1000) do
      c(i.i32())
    end
    c.stop()

Notes, so far:

  • this was gutted out from a much larger codebase, I couldn't find as smaller example
  • It doesn't segfault when:
    • actor Caller is removed and its apply and stop behaviours are implemented in Main
    • foos is an (Array[I32] trn | Array[I32] val) instead of (Array[Foo val] trn | Array[Foo val] val)
    • the Range in Main.create() is small (< 20)
    • _notify.result(foos) in FooGenerator.stop() is not called, even if the debug statement uses foos as a recovered val
  • here I compiled with 0.14.0-f70dba091 [release] llvm 3.9.1 -- cc (GCC) 7.1.1 20170516 but I wrote the code in ponyc 0.10 time and had the segfault then (the code base is large, it took time to narrow down to this)
@lisael lisael changed the title Segfault when using a recoverd trn Segfault when using a recoverd trn Array Jun 24, 2017
@lisael
Copy link
Contributor Author

lisael commented Jun 24, 2017

In my larger code, it sometimes doesn't segfault but in that case the objects in the Array hava inconsistent field values (a random number instead of the expected value). It looks like a bad pointer or use-after-free error.

@jemc jemc added bug: 1 - needs investigation triggers release Major issue that when fixed, results in an "emergency" release labels Jun 24, 2017
@plietar
Copy link
Contributor

plietar commented Jun 24, 2017

This is similar to #1979. (foos as Array[Foo val] trn) should not be allowed, since we can't determine the capability at runtime.

I'm not sure what could be causing the segfault though.

@lisael
Copy link
Contributor Author

lisael commented Jun 24, 2017

I think the error is here:

foos = recover val foos end

foos is possibly not sendable (trn). recover should complain about that but is somewhat confused by the composite type.

My solution here is turning stop() into something much more idiomatic when it comes to trn fields.

class FooGenerator
  var foos: (None | Array[Foo val] trn) = recover trn Array[Foo val] end
  let _notify: Bar val

  fun ref stop() =>
    try
      let foos' = (foos = None) as Array[Foo val] trn
      _notify.result(consume val foos')
    end

  ...

However, the compiler should spot the original programing error.

lisael added a commit to lisael/pony-postgres that referenced this issue Jun 24, 2017
  * remove old-style lanbda functions
  * fix many occurences of ponylang/ponyc#1983
  * fixes in the batch mechanism
@lisael
Copy link
Contributor Author

lisael commented Jun 26, 2017

The code in the issue description shows the resulting segfault. The fact that this much shorter chunk compiles is the real issue.

actor Main
  var foo: (String trn| String val) = recover trn String end
  var bar: (String trn| I32) = recover trn String end

  new create(e: Env) =>
    let foo' = recover val foo end
    // let bar' = recover val bar end

This compiles as long as let bar' = is commented, showing that the problem occurs when a type is a composite of two different capabilities on the same base type, just like in #1979 . This is probably a duplicate, but I can't prove it without digging the codegen/type system code.

@sylvanc
Copy link
Contributor

sylvanc commented Jun 28, 2017

Thanks very much for the excellent diagnosis, @lisael. This is indeed the same as #1979. The plan is to change the way type checking for as works to remove the match sugar and remove the compiler internal consume ! AST.

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

5 participants