Do not initialize caml_parent_stack in caml_interprete

[kayceesrk]

If caml_parent_stack was reset to NULL, then the link between parent and child fiber is severed if the child performs a C call which in turn calls back into OCaml. See the example here [1]. Build the example as make COMP=MULTI. With the fix you should see:

[Caml] Call caml_to_c
[C] Enter caml_to_c
[C] Call c_to_caml
[Caml] Enter c_to_caml
[Caml] Handle effect E. Continuing..
[Caml] Leave c_to_caml
[C] Return from c_to_caml
[C] Leave caml_to_c
[Caml] Return from caml_to_c

and without it:

[Caml] Call caml_to_c
[C] Enter caml_to_c
[C] Call c_to_caml
[Caml] Enter c_to_caml
Fatal error: exception Unhandled

[1] https://github.com/kayceesrk/ocaml-eff-example/tree/master/callbacks

[edit: Added missing link]

[stedolan]

Did you leave out the link "[1]"? I don't see it.

[edited comment, what I wrote didn't make much sense]

Does your example attempt to perform an effect from within a callback to a handler outside the callback, or use a handler entirely within the callback?

[kayceesrk]

Sorry. Edited the comment to include [1]. Example performs callback from within a handler with a callback outside the handler. Specifically, there are intervening C frames. Do you think it is not possible to provide sensible semantics for perform crossing C callbacks?

[stedolan]

Ah, right. No, I think the original behaviour was more correct.

In this example, there is a sensible semantics for the effect, but that's because the effect is so simple - it's essentially a function call. If the effect handler chose to capture the continuation, there's nothing useful we can do.

For instance, if the effect handler captured the continuation, and then ran another fiber which also did C calls and performed a similar effect, and then tried to reinstate the first continuation, we'd be left trying to move C stack frames around (which doesn't work, as C coders fill datastructures with pointers to their stack).

I think we should give a better error message than "Unhandled", though - we don't know whether there's a handler or not, but we give up searching because of a C frame in the way.

[lpw25]

Yes, breaking the parent chain in the original code was deliberate since we cannot capture C stack frames. I also think that raising Unhandled is the correct behaviour.

The semantics of calling into the OCaml runtime from C are to run the OCaml call in its own context with no existing effect (or exception) handlers in place. So an effect performed within such a call should perform its default action (which is currently always raising Unhandled).

[kayceesrk]

Right. I agree. This merge should be reverted.

Regarding the more sane error message, I think this problem case can be distinguished from typical unhandled effect by the fact that the initial_parent_stack is not null when the effect is performed through a C callback.

[yallop]

The semantics of calling into the OCaml runtime from C are to run the OCaml call in its own context with no existing effect (or exception) handlers in place

In the current implementation that appears to be true for effects, but not for exceptions.

[stedolan]

On Thu, Oct 1, 2015 at 11:34 AM, yallop notifications@github.com wrote:

The semantics of calling into the OCaml runtime from C are to run the
OCaml call in its own context with no existing effect (or exception)
handlers in place

In the current implementation that appears to be true for effects, but not
for exceptions.

Correct. C callbacks have to handle the possibility that they get jumped
over. This is doable in C, because the compiler can't assume the absence of
longjmp anyway.

To make effects work, C callbacks would have to handle the possibility of
having stackframes moved, which the compiler assumes doesn't happen.

I really think this should be a separate error from "Unhandled", to avoid
programmer confusion.

[lpw25]

In the current implementation that appears to be true for effects, but not for exceptions.

I thought exceptions raised in the callback were returned to the calling C specially (by setting the second bit).

[lpw25]

I really think this should be a separate error from "Unhandled", to avoid
programmer confusion.

The problem is if there is a sensible default action (for example using blocking read for all the basic read operations) then it really should be executed in this case. If we don't do that then we can't change the Stdlib to use effects as it will destroy backwards compatibility. And I am very reluctant to special-case the "default default".

[lpw25]

I thought exceptions raised in the callback were returned to the calling C specially (by setting the second bit).

Oh I see. I had caml_callback_exn in mind rather than caml_callback.

[lpw25]

Actually, I think we could manage a better message. Once we allow people to set their own default handler then we really don't want to encourage catching Unhandled anyway since they should at the very least raise an effect-specific exception. So I suppose we can make Unhandled be Unhandled of string and put a message inside it containing the name of the effect and something like (from inside caml_callback) where appropriate.

[stedolan]

I like Unhandled of string. We can also shove in the name of the effect,
since that's available at runtime.
On 1 Oct 2015 12:36, "Leo White" notifications@github.com wrote:

Actually, I think we could manage a better message. Once we allow people
to set there own default handler then we really don't want to encourage
catching Unhandled anyway since they should at the very least raise an
effect-specific exception. So I suppose we can make Unhandled be Unhandled
of string and put a message inside it containing the name of the effect
and something like (from inside caml_callback) where appropriate.

—
Reply to this email directly or view it on GitHub
#30 (comment)
.