odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.55k stars 570 forks source link

odin test hangs inconsistently #2498

Closed kleeon closed 3 months ago

kleeon commented 1 year ago

Version

Odin: dev-2023-04:
OS:   Windows 10 Professional (version: 21H2), build 19044.2846
CPU:  AMD Ryzen 5 5600X 6-Core Processor
RAM:  32691 MiB

Context

The following program will hang indefinitely for me:

package main

import "core:fmt"
import "core:testing"

@(test)
test_proc :: proc(t: ^testing.T) {
    fmt.println("This is a test")

    testing.expect(t, true)
}

@(test)
test_proc2 :: proc(t: ^testing.T) {
    fmt.println("This is a test2")

    testing.expect(t, true)
}

main :: proc() {
    tests := make([dynamic]testing.Internal_Test, 0)

    append(&tests, testing.Internal_Test{
        pkg = "main",
        name = "test_proc",
        p = test_proc,
    })

    append(&tests, testing.Internal_Test{
        pkg = "main",
        name = "test_proc2",
        p = test_proc2,
    })

    for { // run until it deadlocks
        testing.runner(tests[:])
    }
}

I rewrote it in the main procedure because it was easier to debug, but it also hangs when running through odin test. I had to delete //+private from runner.odin to make this work.

It takes a little whilte to hang on my main machine(~10 seconds), but it hangs almost immediately on a slower Core 2 Quad PC.

Output is very inconsistent. Looks like a bug related to race condition:

λ odin run test.odin -file
[Package: main]
[Test: test_proc]
This is a test
[test_proc : SUCCESS]
[Test: test_proc2]
This is a test2
[test_proc2 : SUCCESS]
----------------------------------------
2/2 SUCCESSFUL
[Package: main]
[Test: test_proc]
This is a test

I'm not 100% sure about this, but I think this might be due to this win32.ResumeThread call:

https://github.com/odin-lang/Odin/blob/f0ba5d382130800e122776464897671a12f3e7a1/core/testing/runner_windows.odin#L131

The thread doesn't seem to resume properly sometimes and thus global_threaded_runner_semaphore never gets signalled.

Apparently, it's not a good idea to suspend and resume threads like this:

https://devblogs.microsoft.com/oldnewthing/20031209-00/?p=41573

Feoramund commented 3 months ago

This is not relevant anymore, at least for the test runner, as it's been completely rewritten and the Windows-specific code has been removed.

kleeon commented 3 months ago

@Feoramund

I checked it and you are right. Tests seem to be running without any problems now.

Closing the issue.