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

provide CAMLreturn macros suitable for callbacks #4891

Closed
vicuna opened this issue Oct 13, 2009 · 9 comments
Closed

provide CAMLreturn macros suitable for callbacks #4891

vicuna opened this issue Oct 13, 2009 · 9 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented Oct 13, 2009

Original bug ID: 4891
Reporter: @ygrek
Assigned to: @damiendoligez
Status: resolved (set by @mshinwell on 2016-12-07T16:52:06Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 3.11.1
Target version: 4.05.0 +dev/beta1/beta2/beta3/rc1
Category: ~DO NOT USE (was: OCaml general)
Monitored by: yziquel @ygrek

Bug description

Consider callbacks from C to OCaml. Call to caml_callback in the C wrapper code is surrounded with caml_leave_blocking_section and caml_enter_blocking_section. In case when this C callback manipulates ocaml values, it registers them with CAMLlocal. Consequently one should use CAMLreturn to unregister those values. But both CAMLlocal and CAMLreturn should be called with ocaml runtime lock acquired (to prevent race condition on caml_local_roots). But caml_enter_blocking_section cannot be called after CAMLreturn because the latter actually returns from the function. So one has to wrap the C function into another one which solely manages the runtime lock and passes arguments, like this :

static int progressFunction(void *data,
double dlTotal,
double dlNow,
double ulTotal,
double ulNow)
{
int r;
leave_blocking_section();
r = progressFunction_nolock(data,dlTotal,dlNow,ulTotal,ulNow);
enter_blocking_section();
return r;
}

It would be much nicer to have macros like CAML_callback_return and CAML_callback_returnT (and possibly CAML_callback_enter (alias to CAMLparam0?)) to unregister local values, release runtime lock and return.

@vicuna
Copy link
Author

vicuna commented Oct 11, 2010

Comment author: yziquel

Yes. I have the same issue. I do:

caml_local_roots = caml__frame;
caml_enter_blocking_section();
return;

to return from OCaml code into standalone C(++) code.

Moreover, I haven't checked yet, but I'm worried about the caml_local_roots. If caml_local_roots is managed each time there's an OCaml thread context switch, then it's fine. We can just pop up the frames this way.

If not, then how can we be sure that there isn't a race condition? I mean:

-1- thread1: caml_leave_blocking_section
-2- thread1: CAMLparam3();
-3- thread1: caml_callbackN(...);
-4- thread2: CAMLparam2()
-5- thread1: returns from OCaml callback
-6- thread1: caml_local_roots = caml__frame;
...
-15- thread2: CAMLreturn(...)

The order of -2-, -4-, -6-, -15- seems problematic to me.

@vicuna
Copy link
Author

vicuna commented May 20, 2011

Comment author: @damiendoligez

I think it's better to just add a macro to pop the frame without returning.

@vicuna
Copy link
Author

vicuna commented Jul 8, 2012

Comment author: @ygrek

Yes, sounds reasonable.

@vicuna
Copy link
Author

vicuna commented Dec 23, 2013

Comment author: @ygrek

#define CAMLleave() do{
caml_local_roots = caml__frame;
}while (0)

Name comes from the observation that CAMLparam0 enters the scope where OCaml values will be introduced with CAMLlocal macros and CAMLleave explicitly leaves that scope

@vicuna
Copy link
Author

vicuna commented Jun 24, 2015

Comment author: @ygrek

apparently this is fixed by #142

@vicuna
Copy link
Author

vicuna commented Sep 5, 2016

Comment author: goswin

yziquel:

It must be handled by the GC on every thread switch or it wouldn't work. I believe, without checking the source, it must be handled as part of leave/enter blocking section. Makes sense to backup the threads root when entering a blocking section and restoring it when leaving it.

@vicuna
Copy link
Author

vicuna commented Sep 5, 2016

Comment author: goswin

ygrek:

I see the patch adds CAMLdrop() to replace the CAMLreturn0(). But that leaves CAMLparam0(). I don't think it is documented anywhere either way so let me make sure:

Does CAMLparam0() have to be the first think in a function? I assume not.

Is this how CAMLdrop() should be used?

int do_callback(const char *name) {
    int res;
    caml_leave_blocking_section();
    CAMLparam0();
    CAMLlocal2(_name, _res);
    _name = caml_copy_string(name);
    _res = caml_callback1(_func, _name);
    if (Is_exception_result(_res)) {
        // Extract_exception(_res)
    }
    res = Int_val(res);
    CAMLdrop();
    caml_enter_blocking_section();
    return res;
}

I still think the caml_leave_blocking_section() could be combined with CAMLparam0() and maybe even CAMLlocalX() as mentioned in #7340. Same for CAMLdrop() and caml_enter_blocking_section(). This is a pattern every single callback to ocaml needs to repeat.

@vicuna
Copy link
Author

vicuna commented Sep 26, 2016

Comment author: @ygrek

I think the intention was to keep things explicit and I am happy with current solution. See e.g. ygrek/ocurl@bfa4285

@vicuna
Copy link
Author

vicuna commented Dec 7, 2016

Comment author: @mshinwell

Any further discussion can go to #7340 since the original poster's wishes have been satisfied here, as far as I can tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants