realworldocaml / mdx

Execute code blocks inside your documentation
ISC License
265 stars 45 forks source link

Race in MDX output buffering? #424

Closed talex5 closed 1 year ago

talex5 commented 1 year ago

Since updating to MDX 2.3.0, I sometimes see output appearing in the wrong place. For example, I just got this with the Eio tests:

dune build @runtest @all
File "lib_eio/tests/ctf.md", line 1, characters 0-0:
------ lib_eio/tests/ctf.md
++++++ lib_eio/tests/.mdx/ctf.md.corrected
File "lib_eio/tests/ctf.md", line 1, characters 0-1:
+|1025
+|1026
+|1027
+|1028
 |# Test unique ID generation
 |
 |
 |```ocaml
 |# #require "eio";;
 |# for _ = 1 to 5 do
 |    Printf.printf "%d\n%!" (Eio.Private.Ctf.mint_id () :> int)
 |  done;;
 |1
 |2
 |3
 |4
 |5
 |- : unit = ()
 |```
 |
 |A new domain gets a new chunk:
 |
 |```ocaml
 |# Domain.spawn
 |    (fun () ->
 |      for _ = 1 to 5 do
 |        Printf.printf "%d\n%!" (Eio.Private.Ctf.mint_id () :> int)
 |      done);;
 |1024
-|1025
-|1026
-|1027
-|1028
 |- : unit Domain.t = <abstr>
 |```
 |
 |When the original domain exhausts its chunk, it jumps to the next free chunk:
 |
 |```ocaml
 |# for _ = 1 to 1024 - 9 do
 |    Eio.Private.Ctf.mint_id () |> ignore
 |  done;;
 |- : unit = ()
 |
 |# for _ = 1 to 5 do
 |    Printf.printf "%d\n%!" (Eio.Private.Ctf.mint_id () :> int)
 |  done;;
 |1021
 |1022

Here, MDX seems to want to insert some output right at the start of the file, before any code blocks!

I haven't investigated further - has anything changed recently that might cause this?

Leonidas-from-XIV commented 1 year ago

I haven't investigated further - has anything changed recently that might cause this?

The diff between 2.2.1 and 2.3.0 is pretty short and the only thing I can think of is 47388802930d4b13cf6bfc904660530f3c614ed6 and 00701ac7e290c87af29cfcec8f8aa7d51cc1a818 where I replaced constructing strings in memory with formatters. You could try reverting these commits and see if it solves the issue.

This makes me a bit unhappy, since that shows that the code is surprisingly fragile and has triggered some race condition.

talex5 commented 1 year ago

I've now seen this with 2.2.1 too, so I guess it was something else.

talex5 commented 1 year ago

I suspect this is something to do with seeking after the block has finished and not preventing it from continuing to write. The failing test-case is missing a Domain.join, so it may still be running then. Here's a simpler version:

```ocaml
# #require "unix";;

# Domain.spawn
    (fun () ->
      for i = 1 to 20 do
        Unix.sleepf 0.0001;
        Printf.printf "%d\n%!" i
      done);;
```

Produces (you might have to play with the timing):

8
9
```ocaml
# #require "unix";;

# Domain.spawn
    (fun () ->
      for i = 1 to 20 do
        Unix.sleepf 0.0001;
        Printf.printf "%d\n%!" i
      done);;
1
2
3
4
5
6
- : unit Domain.t = <abstr>
7
```
10
talex5 commented 1 year ago

Closing as I guess it's not worth protecting against this in MDX.