🐛 [Bug]: Immutable configuration in Fiber not working as expected #3236
Assignees
Labels
Comments
|
This is probably happening in v3 too. |
9 tasks
|
@gaby there’s also a data race occurring when storing a reference to Is there a way to determine if the request has been closed and the fiber.Ctx is no longer valid? |
|
@rebaz94 We probably missing it somewhere else. |
|
@rebaz94 Can you share the logs of the data race? |
|
Apologies for the delayed response. Here's a video demonstrating how you can reproduce the issue: Screen.Recording.2024-12-22.at.2.04.59.AM.mp4Here the logs |
|
@rebaz94 Found the issue, it's related to |
|
I can take a look at this. @gaby |
|
@aliziyacevik I have the fix, forgot to submit a PR. |
|
@aliziyacevik Feel free to submit a PR, basically we need the implementation of |
If you have the fix, I think you should submit it. I haven't got the chance the look at this issue now. |
|
Is this error still happening? I’ve been trying to help by reproducing the error, and I’ve made several requests based on the code provided, but I haven’t been able to replicate the issue. I tested with both version v2.52.5 and v3, following the steps and making requests as described, but the error doesn’t seem to appear on my end. |
|
@rebaz94 Can you test using github.com/gofiber/fiber/v2@master |
|
it should be fixed with |
|
you can also test with https://github.com/gofiber/fiber/releases/tag/v2.52.6 |
9 tasks
|
I tried it with the latest version, but the issue is still the same. |
|
I just tested it without calling any function from ctx, and it shows a data race! s.app.Get("/test", func(ctx *fiber.Ctx) error {
return nil // add a breakpoint here
}) |
|
@rebaz94 I see what you mean, but you have a bunch of other middlewares in your code. I think those middlewares are the ones not respecting the Which middlewares are you using, so I can take a look at each one. |
|
@ReneWerner87 I think this exposes a bug, when you do: app := fiber.New(fiber.Config{
Immutable: true,
StreamRequestBody: true,
})Only the core is respecting that |
|
I see that you are using:
|
|
Yes I use these middlewares:
Then I removed all middlewares and still happen: full logs |
|
@rebaz94 Ok very interesting |
|
After testing, I found that Go wasn't updating the package properly. The logs showed version v2.52.6, but the old code was still in use. After updating, there were no more race conditions when accessing the body after the handler returns or using middleware. However, a race condition still occurs when the IDE tries to display ctx, triggering ctx.String(): WARNING: DATA RACE
Read at 0x00c00071afa8 by goroutine 68:
github.com/valyala/fasthttp.(*URI).FullURI()
./app/pkg/mod/github.com/valyala/fasthttp@v1.58.0/uri.go:810 +0x40
github.com/gofiber/fiber/v2.(*Ctx).String()
./app/pkg/mod/github.com/gofiber/fiber/v2@v2.52.6/ctx.go:1843 +0x114
|
|
@rebaz94 Thanks for sharing, will check tonight again. That's progress |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug Description
The
Immutableconfiguration option in Fiber, when set to true, is intended to ensure that certain request values (e.g., request bodies) are immutable and accessible beyond the handler's lifecycle. However, enabling this option does not work as expected, resulting in data race errors when the request body is processed within a differernt goroutine.Documentation Says:
Am I missing something here, or is this the expected behavior, and I need to manually copy the body to ensure immutability in my code?
How to Reproduce
Expected Behavior
The request body
c.Body()should remain immutable and safe to access across goroutines without causing data races but actually get a data racesFiber Version
v2.52.5
Checklist:
The text was updated successfully, but these errors were encountered: