rgrinberg / opium

Sinatra like web toolkit for OCaml
MIT License
755 stars 67 forks source link

Exception raised in custom environments - Exception not catch, raised by "input_line", App.ml, l. 106 #288

Open aeghost opened 1 year ago

aeghost commented 1 year ago

Hi.

I used Opium with OpenAPI addon, to make a prototype locally, in normal Fedora/Linux environment, that works wonderfully, but when deployed on targeted system, binary crashed because End_of_file exception.

After some investigations, it comes from App.ml, l. 106.

let system_cores =
  match Sys.unix with
  | false ->
    (* TODO: detect number of cores on Windows *)
    1
  | true ->
    let ic = Unix.open_process_in "getconf _NPROCESSORS_ONLN" in
    let cores = int_of_string (input_line ic) in
    ignore (Unix.close_process_in ic);
    cores
;;

Opium is deployed in a minimalist chroot, on a custom build Linux environment with no rights.

It means, that there is no getconf (input_line raised End_of_file), and no shell accessible for any binaries for security reasons (so Unix.open_process_in does not work either, even with getconf accessible).

I would propose an evolution, or at least a "normal" patch that would consist in catching ALL unexposed, raised exception.

In this case, I would propose.

let system_cores =
  match Sys.unix with
  | false ->
    (* TODO: detect number of cores on Windows *)
    1
  | true -> begin try
        let ic = Unix.open_process_in "getconf _NPROCESSORS_ONLN" in
        let cores = int_of_string (input_line ic) in
        ignore (Unix.close_process_in ic);
        cores
      with End_of_file ->
        (* MAYBE: Warn user that you can't determine how many cores are available *)
        1
    end

;;

Or maybe add a ?(available_cores = 1) parameter in server options that would let the user define how many cores Webserver should use.

But the only solution we found is using a proxy of Opium just to patch this, which is not a valid long term solution ;D. So hopping it deserves a patch and a 0.21.0.

By wishing you a good day,

Matthieu GOSSET

shawnanastasio commented 1 year ago

I'm also encountering this on OpenBSD which doesn't seem to support _NPROCESSORS_ONLN. Wrapping the input_line call in a try/with as described above works around the issue. It would be great to see this fixed upstream, and I could submit a pull request if desired.