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

Race in MDX output buffering? #424

Closed
talex5 opened this issue Apr 18, 2023 · 4 comments
Closed

Race in MDX output buffering? #424

talex5 opened this issue Apr 18, 2023 · 4 comments

Comments

@talex5
Copy link
Contributor

talex5 commented Apr 18, 2023

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
Copy link
Collaborator

Leonidas-from-XIV commented Apr 18, 2023

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 4738880 and 00701ac 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
Copy link
Contributor Author

talex5 commented Apr 19, 2023

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

@talex5
Copy link
Contributor Author

talex5 commented Apr 20, 2023

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
Copy link
Contributor Author

talex5 commented Apr 21, 2023

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

@talex5 talex5 closed this as completed Apr 21, 2023
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