Skip to content

Commit

Permalink
Remove Lazy from native and bytecode modes: not thread safe
Browse files Browse the repository at this point in the history
Angstrom (and as a consequence Uri.of_string) is not thread-safe.
On OCaml 4 this actually leads to a SIGSEGV sometimes, on OCaml 5 it raises CamlinternalLazy.Undefined.

This could be fixed by adding a 'Lazy.force p |> ignore' so that the Lazy value is never observable from a concurrent/parallel context,
but I prefer to remove Lazy completely, at least until the reason for the OCaml 4 segfault is understood.

Using a ref here is safe, because the ref is only changed once before 'fix_direct' returns, so it is never changed from a concurrent/parallel context.
And it still preserves the optimization that Lazy did: the fixpoint is only computed once.

This avoids the crash noticed in mirage/ocaml-uri#178

Use @gasche's suggestion to avoid an option by storing a function that would raise initially
(this function never gets invoked, because the ref is immediately updated)

Signed-off-by: Edwin Török <[email protected]>
  • Loading branch information
edwintorok committed Sep 11, 2024
1 parent aff4665 commit 181f1a0
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions lib/angstrom.ml
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,14 @@ let take_till f =
let choice ?(failure_msg="no more choices") ps =
List.fold_right (<|>) ps (fail failure_msg)

let notset = { run = fun _buf _pos _more _fail _succ -> failwith "Angstrom.fix_direct not set" }

let fix_direct f =
let rec p = lazy (f r)
let rec p = ref notset
and r = { run = fun buf pos more fail succ ->
(Lazy.force p).run buf pos more fail succ }
(!p).run buf pos more fail succ }
in
p := f r;
r

let fix_lazy ~max_steps f =
Expand Down

0 comments on commit 181f1a0

Please sign in to comment.