go-fuego / fuego

Golang Fuego - web framework generating OpenAPI 3 spec from source code
https://go-fuego.github.io/fuego/
MIT License
918 stars 46 forks source link

Data race due to global instance of openapi3gen.Generator #201

Closed loikg closed 4 weeks ago

loikg commented 1 month ago

To Reproduce Create multiple instance of fuego.Server in different goroutines. I have create a simple repository to showcase the problem over here. But for sake of completeness here the test used:

package main

import (
    "fmt"
    "testing"

    "github.com/go-fuego/fuego"
)

func TestDataRace(t *testing.T) {
    t.Parallel()

    for n := range 2 {
        t.Run(fmt.Sprintf("create server %d", n), func(t *testing.T) {
            t.Parallel()
            createFuegoServer(t)
        })
    }
}

func createFuegoServer(t *testing.T) {
    t.Helper()

    server := fuego.NewServer()
    fuego.Get(server, "/", func(ctx *fuego.ContextNoBody) (any, error) {
        return nil, nil
    })
}

Expected behavior The data race should not happen.

Framework version: Tested on v0.14.0 and main at the time of writing (f386d4f).

Go version: v1.23.2

Additional context This prevent running tests that exercise the entire server in parallel, but does not affect unit test of handlers as far as I understand. The root cause is the package global variable generator at line 151 in openapi.go being shared and concurrently used by every fuego.Server instances. See #202 for a potential fix.

Huge thanks for your work, I really enjoy using fuego!

EwenQuim commented 1 month ago

Wow I didn't thought that someone would use several instances of fuego.Server haha.

Just a side question : why do you need that?

Thanks for raising this issue :)

loikg commented 1 month ago

No worries. The reason is purely to be able to run tests in parallel.

I took the habit of making every test that I can run in parallel with t.Parallel() and in some of my tests I want to test the entire "stack" including middlewares that should be trigger when calling a specific endpoint, thus these tests create multiple fuego.Server instances in parallel leading to the data race.

EwenQuim commented 1 month ago

Very good idea indeed