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

Runtime::init is broken if called multiple times #39

Open
mrmr1993 opened this issue Aug 16, 2021 · 3 comments
Open

Runtime::init is broken if called multiple times #39

mrmr1993 opened this issue Aug 16, 2021 · 3 comments

Comments

@mrmr1993
Copy link

For example,

fn call_first() {
  let r1 = Runtime::init();
  ...
}
fn call_snd() {
  let r2 = Runtime::init();
  ...
}
pub fn main() {
  call_first();
  call_snd();
}

Currently, the second call will be unable to use the OCaml runtime, because it has been shutdown at the end of the first call.

It seems caml_startup and caml_shutdown already handle this correctly: startup increments a counter, shutdown decrements it, and the runtime is only started or stopped when that counter is 0. Removing the std::sync::Once in Runtime::init_persistent would restore this built-in behaviour, which seems to be strictly an improvement and fix this issue.

@tizoc
Copy link
Owner

tizoc commented Aug 16, 2021

@mrmr1993 that is correct, but it is exactly the behavior you get from caml_startup and caml_shutdown from the OCaml runtime, and I don't think there is a good way to fix it. BUT an improvement here would be to track that a shutdown has happened and panic if init is called again.

See here at the end of the section:

Once a runtime is unloaded, it cannot be started up again without reloading the shared library and reinitializing its static data. Therefore, at the moment, the facility is only useful for building reloadable shared libraries.

@tizoc
Copy link
Owner

tizoc commented Aug 16, 2021

I am even thinking that there is no need to track shutdowns, calling init more than once is just an error here, because you shouldn't ever have more than one owned instance of the runtime handle. So any call to init() after the first should panic.

@tizoc
Copy link
Owner

tizoc commented Aug 16, 2021

I guess it should better return a Result instead of triggering a panic to let the caller decide what to do. It is an api-breaking change so probably good to leave that to the split if things end being done that way.

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

No branches or pull requests

2 participants