valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.7k stars 1.75k forks source link

Goroutine race while using *fasthttp.RequestCtx #1876

Closed DaryaHom closed 1 day ago

DaryaHom commented 2 days ago

Hello! I am using fasthttp.RequestCtx. The documentation states that “It is unsafe modifying/reading RequestCtx instance from concurrently running goroutines.” I find this confirmed in my test. But maybe there is some safe use case if I need to run multiple goroutines? Please help me figure out how to fix this. Thank you

My code snippet:

package main

import (
    "context"
    "time"

    "github.com/valyala/fasthttp"
)

//go:generate go install github.com/vektra/mockery/v2@v2.43.0
//go:generate mockery --dir=./ --all --output=./mocks --outpkg=mocks --case=underscore --exported=true
type FirstTestRepo interface {
    Get(ctx context.Context)
}

type SecondTestRepo interface {
    Get(ctx context.Context)
}

type ThirdTestRepo interface {
    Get(ctx context.Context)
}

type Repo struct{}

func (r *Repo) Get(ctx context.Context) {
    _ = ctx
}

func MakeWork(ctx context.Context, r1 FirstTestRepo, r2 SecondTestRepo, r3 ThirdTestRepo) error {
    for i := 0; i < 10; i++ {
        go parallelWork(ctx, func(ctx context.Context) {
            r1.Get(ctx)
            r2.Get(ctx)
            r3.Get(ctx)
        })
    }
    return nil
}

func parallelWork(ctx context.Context, f func(ctx context.Context)) {
    f(ctx)
}

func main() {
    firstRepo := &Repo{}
    secondRepo := &Repo{}
    thirdRepo := &Repo{}
    ctx := &fasthttp.RequestCtx{}
    _ = MakeWork(ctx, firstRepo, secondRepo, thirdRepo)

    time.Sleep(10 * time.Second)
}

The test:

package main

import (
    "testing"

    "tests/mocks"

    "github.com/stretchr/testify/mock"
    "github.com/valyala/fasthttp"
)

func TestMakeWork(t *testing.T) {
    tests := []struct {
        name    string
        ctx     *fasthttp.RequestCtx
        makeSut func(t *testing.T, ctx *fasthttp.RequestCtx) (
            r1 FirstTestRepo,
            r2 SecondTestRepo,
            r3 ThirdTestRepo,
        )
        wantErr bool
    }{
        {
            name: "1",
            ctx:  &fasthttp.RequestCtx{},
            makeSut: func(t *testing.T, ctx *fasthttp.RequestCtx) (
                FirstTestRepo,
                SecondTestRepo,
                ThirdTestRepo,
            ) {
                r1 := mocks.NewFirstTestRepo(t)
                r2 := mocks.NewSecondTestRepo(t)
                r3 := mocks.NewThirdTestRepo(t)

                r1.On("Get", mock.Anything)
                r2.On("Get", mock.Anything)
                r3.On("Get", mock.Anything)
                return r1, r2, r3
            },
            wantErr: false,
        },
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            r1, r2, r3 := tt.makeSut(t, tt.ctx)
            if err := MakeWork(tt.ctx, r1, r2, r3); (err != nil) != tt.wantErr {
                t.Errorf("MakeWork() error = %v, wantErr %v", err, tt.wantErr)
            }
        })
    }
}

Test result:

WARNING: DATA RACE
Write at 0x00c00016a240 by goroutine 11:
  github.com/valyala/fasthttp.(*URI).FullURI()
      /home/daria/go/pkg/mod/github.com/valyala/fasthttp@v1.53.0/uri.go:820 +0x1f5
  github.com/valyala/fasthttp.(*RequestCtx).String()
      /home/daria/go/pkg/mod/github.com/valyala/fasthttp@v1.53.0/server.go:891 +0x148
  fmt.(*pp).handleMethods()
      /usr/local/go/src/fmt/print.go:673 +0x6ea
  fmt.(*pp).printArg()
      /usr/local/go/src/fmt/print.go:756 +0xb4b
  fmt.(*pp).doPrintf()
      /usr/local/go/src/fmt/print.go:1174 +0x10ce
  fmt.Sprintf()
      /usr/local/go/src/fmt/print.go:239 +0x5c
  github.com/stretchr/testify/mock.Arguments.Diff()
      /home/daria/go/pkg/mod/github.com/stretchr/testify@v1.9.0/mock/mock.go:939 +0x1b2
  github.com/stretchr/testify/mock.(*Mock).findExpectedCall()
      /home/daria/go/pkg/mod/github.com/stretchr/testify@v1.9.0/mock/mock.go:368 +0x147
  github.com/stretchr/testify/mock.(*Mock).MethodCalled()
      /home/daria/go/pkg/mod/github.com/stretchr/testify@v1.9.0/mock/mock.go:476 +0xac
  github.com/stretchr/testify/mock.(*Mock).Called()
      /home/daria/go/pkg/mod/github.com/stretchr/testify@v1.9.0/mock/mock.go:466 +0x195
  tests/mocks.(*FirstTestRepo).Get()
      /home/daria/Документы/test/mocks/first_test_repo.go:18 +0xb5
  tests.MakeWork.func1()
      /home/daria/Документы/test/main.go:33 +0x78
  tests.parallelWork()
      /home/daria/Документы/test/main.go:42 +0x4f
  tests.MakeWork.gowrap1()
      /home/daria/Документы/test/main.go:32 +0x17

fasthttp version - v1.53.0

Actions for playback:

  1. go generate ./...
  2. go test ./... -count=1 -race
newacorn commented 2 days ago

Using fasthttp.RequestCtx is similar to working with a regular struct. Its fields should not be accessed concurrently, as access to them is not protected by a mutex. It's recommended that you provide specific use cases, as offering generic solutions tends to be inefficient and already well-known.

erikdubbelboer commented 1 day ago

I don't know what you want to do with the fasthttp.RequestCtx, but normally you wouldn't really use this struct directly. I also wouldn't recommend using it as context.Context. To be honest, I would recommend just using net/http for whatever you want to do.