Ponyc: Segmentation fault when actor receives a reference to itself via a class created in a different actor.

[Perelandric]

_EDIT:_ Skip down to https://github.com/ponylang/ponyc/issues/1118#issuecomment-238431412 to see the most reduced example of the issue.

---

I gutted the HTTP server down to this, which I think is a reproduction of the seg fault in #937. I kept the original type names so that they could be somewhat related back to that package if necessary. Seems to have something to do with the partial function this~answer(). At least if I interrupt anything after that assignment, it the seg fault disappeared.

interface val ResponseHandler
  fun val apply(request: Payload val, response: Payload val): Any

interface val RequestHandler
  fun val apply(request: Payload): Any


primitive Handle
  fun val apply(request: Payload) =>
    (consume request).respond(Payload)


actor _ServerConnection
  let _handler: RequestHandler

  new create(h: RequestHandler) => _handler = h

  be dispatch(request: Payload) =>
    request.handler = recover this~answer() end
    _handler(consume request)

  be answer(request: Payload val, response: Payload val) =>
    None


class iso Payload
  var handler: (ResponseHandler | None) = None

  fun iso respond(response': Payload) =>
    try
      let h = (handler) as ResponseHandler
      h(consume this, consume response')
    end


actor Main
  new create(env: Env) =>
    let t = Test

    for i in Range(0, 10_000) do
      t.do_it()
    end


actor Test
  be do_it() =>
    _ServerConnection(Handle).dispatch(Payload)

This could probably be further reduced, but I wanted to maintain at least a slight semblance to the original code... and it's the middle of the night so I'm going to :sleeping:.

[Perelandric]

Here's a further reduced example:

interface val ResponseHandler
  fun val apply(request: Payload val): Any


actor _ServerConnection
  be dispatch(request: Payload) =>
    request.handler = recover this~answer() end
    try
      let h = request.handler as ResponseHandler
      h(consume request)
    end

  be answer(request: Payload val) => None


class iso Payload
  var handler: (ResponseHandler | None) = None


actor Main
  new create(env: Env) =>
    let t = Test

    for i in Range(0, 10_000) do
      t.do_it()
    end


actor Test
  be do_it() =>
    _ServerConnection.dispatch(Payload)

If I change the dispatch method to this, the seg fault is gone:

  be dispatch(request: Payload) =>
    let x: (ResponseHandler | None) = recover this~answer() end
    try
      let h = x as ResponseHandler
      h(consume request)
    end

but if I leave off the : (ResponseHandler | None) on the x declaration, it complains of capture violations. If I then make it recover to val, it compiles and the seg fault is again gone.

    let x = recover val this~answer() end

---

Or if I simply set request.handler = None after the let h assignment, the seg fault is gone.

actor _ServerConnection
  be dispatch(request: Payload) =>
    request.handler = recover this~answer() end
    try
      let h = request.handler as ResponseHandler
      request.handler = None
      h(consume request)
    end

So it seems like it has something to do with sending the _ServerConnection into its own .answer() method via the partial function that captures this and gets assigned to the Payload.

Unfortunately, adding handler = None in the respond method in net/http/payload.pony does not fix it. :confused:

[Perelandric]

Given that last bit of info, I can reduce it just a bit more:

interface val ResponseHandler
  fun val apply(request: Payload val): Any


actor _ServerConnection
  be dispatch(request: Payload) =>
    request.handler = recover this~answer() end
    this.answer(consume request)

  be answer(request: Payload val) => None


class iso Payload
  var handler: (ResponseHandler | None) = None


actor Main
  new create(env: Env) =>
    let t = Test

    for i in Range(0, 10_000) do
      t.do_it()
    end


actor Test
  be do_it() =>
    _ServerConnection.dispatch(Payload)

Note that if I create the Payload directly in dispatch instead of passing it from do_it(), we don't get the seg fault. Same if I eliminate _ServerConnection and put all its methods right in Test.

I tried to see if I could create the seg fault by passing the partial function directly to answer instead of via Payload, but wasn't able to get the type quite right for the parameter.

[Perelandric]

We really don't need the partial function at all. The same seg fault can be found by having Payload directly reference the _ServerConnection object.

actor _ServerConnection
  be dispatch(request: Payload) =>
    request.handler = recover this end
    this.answer(consume request)

  be answer(handler: Payload val) => None


class iso Payload
  var handler: (_ServerConnection | None) = None

[SeanTAllen]

So this last 8 line example is enough on its own to cause a segfault? Nicely narrowing of the issue down @Perelandric

[Perelandric]

@SeanTAllen Thanks, and yes, with the Main and Test actors from the example above it, it should reproduce the seg fault. I'll put a full version of the smallest example in this comment.

It'll be interesting to see if this actually fixes the httpserver example. There was some other odd behavior that I noted in issue #937 which is where my investigation began, but this is certainly a piece of the puzzle if not the entire thing.

---

Here's the most simplified yet complete code that I could find to reproduce the issue:

actor Main
  new create(env: Env) =>
    let t = Test

    for i in Range(0, 10_000) do
      t.do_it()
    end


actor Test
  be do_it() => _ServerConnection.dispatch(Payload)


class iso Payload
  var handler: (_ServerConnection | None) = None


actor _ServerConnection
  be dispatch(p: Payload) =>
    p.handler = recover this end
    this.answer(consume p) // <-- Pass the Payload that references this _ServerConnection

  be answer(p: Payload val) => None

Notes:

• In the more complete examples, the _ServerConnection is assigned to the Payload via a partial function binding instead of being assigned directly.
• If the Payload object is created directly in dispatch() instead of at the call location in do_it(), there's no seg fault.
• If you assign None to the .handler field instead of the _ServerConnection object, there's no seg fault.
• The seg fault is intermittent. It's more likely to happen using a Range with 10_000, than with 100.

[SeanTAllen]

Have you tried running a debug version in LLDB to see what is causing the segfault?

[Perelandric]

@SeanTAllen Never used LLDB but this'll be as good a reason as any to tinker. I'll see if I can give it a shot tomorrow.

---

_EDIT:_ Output from my first, mostly blind use of LLDB

(lldb) target create "./test"
Current executable set to './test' (x86_64).
(lldb) run
Process 18472 launched: './test' (x86_64)
Process 18472 exited with status = 0 (0x00000000) 
(lldb) run
Process 18485 launched: './test' (x86_64)
Process 18485 stopped
* thread #2: tid = 18490, 0x0000000000408063 test`pony_traceunknown + 3, name = 'test', stop reason = signal SIGSEGV: invalid address (fault address: 0x40)
    frame #0: 0x0000000000408063 test`pony_traceunknown + 3
test`pony_traceunknown:
->  0x408063 <+3>:  cmpq   $0x0, 0x40(%rax)
    0x408068 <+8>:  je     0x408070                  ; <+16>
    0x40806a <+10>: movq   0x18(%rdi), %rax
    0x40806e <+14>: jmpq   *%rax
(lldb) bt
* thread #2: tid = 18490, 0x0000000000408063 test`pony_traceunknown + 3, name = 'test', stop reason = signal SIGSEGV: invalid address (fault address: 0x40)
  * frame #0: 0x0000000000408063 test`pony_traceunknown + 3
    frame #1: 0x00000000004020ef test`Payload_Trace + 15
    frame #2: 0x000000000040e360 test`ponyint_gc_handlestack + 64
    frame #3: 0x0000000000407fa0 test`ponyint_mark_done + 32
    frame #4: 0x0000000000409304 test`ponyint_actor_run + 772
    frame #5: 0x0000000000405b85 test`run + 69
    frame #6: 0x0000000000405edd test`run_thread + 29
    frame #7: 0x00007ffff7bc4184 libpthread.so.0`start_thread + 196
    frame #8: 0x00007ffff74d737d libc.so.6`clone + 109
(lldb) 

Had trouble setting breakpoints.

[Perelandric]

@SeanTAllen One more observation: It seems if the parameter of the answer() method is tag or iso, there's no segfault. Only a problem with val.

// -------------------v--- iso and tag are OK, but not val
be answer(p: Payload val) => None

[peterzandbergen]

Just out of curiosity, is there any news on this one? After watching the VUG about garbage collection I wondered if the error should be looked for in that area. Just a wild guess.

[SeanTAllen]

Nothing yet.

[peterzandbergen]

Difficult one I guess

[SeanTAllen]

And we could use more folks contributing. Its hard to keep up and do the day job and all that. Such is things... ¯_(ツ)_/¯

[aturley]

Hrm ... I just tried @Perelandric's minimal example from August 8 and I didn't get a seg fault. I tried both a debug and release build on OS X (10.11.4) using compiler version 0.3.0-14-gd8c6c55.

[SeanTAllen]

@perelandric can you see if this still happens for you?

[Perelandric]

@SeanTAllen Yep, took a few extra runs before it would segfault, but it got there.

@aturley Try upping the range from 10_000 to 100_000. That gives me the segfault more reliably.

[aturley]

I got a little more information from lldb:

(lldb) run
Process 12481 launched: './build/issue-1118' (x86_64)
Process 12481 stopped
* thread #2: tid = 0x3422ef, 0x000000010000b862 issue-1118`pony_traceunknown(ctx=0x00000001097ffc48, p=0x00000001217dd000, m=2) + 34 at trace.c:114, stop reason = EXC_BAD_ACCESS (code=1, address=0x40)
    frame #0: 0x000000010000b862 issue-1118`pony_traceunknown(ctx=0x00000001097ffc48, p=0x00000001217dd000, m=2) + 34 at trace.c:114
   111  {
   112    pony_type_t* t = *(pony_type_t**)p;
   113 
-> 114    if(t->dispatch != NULL)
   115    {
   116      ctx->trace_actor(ctx, (pony_actor_t*)p);
   117    } else {

Something is going a little sideways here in GC land.

[SeanTAllen]

Yeah. At the moment, this is waiting on @sylvanc or I having time to address. At the moment, I'm working on new website before returning back to the land of pony code. I'm not going to get to this til sometime in mid October. If anyone wants to take a shot at it before then, have at it.

[dimiii]

Hi, seems like by a happy coincidence it's just a simple typo.

-  pony_type_t* t = *(pony_type_t**)p;
+  pony_type_t* t = (pony_type_t*)p;

After a quick fix all is fine. Here is the cure https://github.com/ponylang/ponyc/pull/1249

[SeanTAllen]

@dimiii all CI jobs failed with a segfault with that change applied. I left some questions for you on #1249.

[Praetonus]

It looks like this is an uninitialised type descriptor pointer problem. I'll try digging into it.

[Praetonus]

I think I got it. This is a premature free issue.

Full tracing of val objects sent in messages is deferred until a GC cycle on the owner. This means the _ServerConnection assigned to the Payload and then sent in a message (via the Payload) has a reference count greater than 0 but isn't aware of it until the Payload is traced. By then, the _ServerConnection might have been GC'd, which of course results in bad stuff happening.

[dimiii]

@Praetonus
@SeanTAllen

BTW, Do you have plans to bootstrap the compiler?
I mean the compiler and runtime.

[jemc]

@dimiii - Building a Pony compiler in Pony is a long-term goal, though it represents a major amount of work. There are no plans to write the runtime in Pony, as it is built on lower-level constructs that would be impractical to provide in Pony.

[SeanTAllen]

@dimiii re: the runtime point @jemc from a safety standpoint, a Pony runtime written in something like Rust would be awesome. However, C is far more accessible to the community and as such, the plans are to stay with C for the forseeable future for the runtime.

[SeanTAllen]

@Praetonus should this be relabeled from "ready for work"?

[Praetonus]

I'm not sure. I can see a fix for this but I think it would have some performance penalty. Maybe removing it until we've discussed it on the sync call would make sense.

[peterzandbergen]

Stability should be more important, imho. I would not advocate using a tool for server applications if it could occasionally crash.

[peterzandbergen]

Especially when the design of the language is to eliminate crashes caused by the programmer.

[SeanTAllen]

no one is suggesting not fixing. the concern is, can we find a way to fix without having a performance impact.

[jemc]

@peterzandbergen - it definitely still needs to be fixed, and it's still marked as high priority. Runtime segmentation faults are not acceptable .

[SeanTAllen]

Ok, issue located. @sylvanc will be working on this. Thank you everyone who contributed to finding this. Extra props to @Perelandric for lots of narrowing down a minimal case and @Praetonus for digging into the code.

[sylvanc]

Yes, thank you VERY much! This is an important and subtle and tricky one.

[peterzandbergen]

Well done, never doubted that this would not be picked up in a serious way. Kudos

[SeanTAllen]

I'm trying to verify that #1507 fixes this issue but I'm unable to reproduce the reported problem.

OSX El Capitan.
LLVM 3.9.1

@Praetonus @Perelandric @aturley can any of you reproduce?

[jkleiser]

Both the original code and the most simplified code here give me "can't find declaration of 'Range'". Why?

[Praetonus]

@jkleiser You have to add use "collections" at the beginning of the code.

[jkleiser]

Thanks @Praetonus . The simplified code now got compiled and ran without problems on my Mac with ponyc 0.10.0-74555f7.

[moesol]

The simplified code works on my Ubuntu machine. Both optimized and with ponyc --debug.

0.10.0-1c33065 [release]
compiled with: llvm 3.9.0 -- gcc-5 (Ubuntu 5.4.1-2ubuntu1~14.04) 5.4.1 20160904

However, when I run the original code I get Segmentation fault (core dumped)

[peterzandbergen]

I have noticed that the issue did not occur in my environment anymore,
being Ubuntu Mate in virtual box. Will test again this week to seeing if
the problem has been resolved.

Op ma 23 jan. 2017 07:54 schreef Robert Hastings [e-mail]:

The simplified code works on my Ubuntu machine. Both optimized and with
ponyc --debug.

0.10.0-1c33065 [release]
compiled with: llvm 3.9.0 -- gcc-5 (Ubuntu 5.4.1-2ubuntu1~14.04) 5.4.1
20160904

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ponylang/ponyc/issues/1118#issuecomment-274413255,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI2XrpZfLdrKh8_toJs6Mh3PLyCqZ6Vhks5rVE6_gaJpZM4JeyOG
.

>

Peter Zandbergen
Tel: [phone]

linkedIn: peterzandbergen https://www.linkedin.com/in/peterzandbergen

[Perelandric]

@peterzandbergen See the discussion at: https://github.com/ponylang/ponyc/pull/1507

[SeanTAllen]

Just an update. @sylvanc is still working on a fix for this.

[SeanTAllen]

Original version still segfaults as of 0.21.0

[SeanTAllen]

The original version of this segfaults if I add some @printf output that slows everything down some. But, only when doing an optimized build.

0.32.0-a7a80758 [release]
compiled with: llvm 7.0.1 -- cc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
Defaults: pic=true