-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
🐛 [Bug]: Very rare panics in ctx.BodyParser or in fasthttp under unknown conditions #2757
Comments
Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
Additionally, maybe there is no bug, can my additional context with Timeout affect when go func() and the second call to ctx.Body -> fasthttp.Request is executed after my context is canceled by timeout, and fiber.Handler has already returned - fiber.Handler { ? I can remove my extra context with timeout and go func() too, because I don't really need it anymore. |
I recently came across a similar problem. It seems that |
can confirm it, no copy is made with the body method, even if immutable is true, if necessary we could correct this another comment is that in a concurrent function, after the end of the run (timeout), an attempt is made to fetch the content of body is also critical |
It would be great if there was an additional option in Fiber or a fix. |
The problem in panic when What should be done to prevent the application from panic? Since fasthttp clearly states that there is no recover in fasthttp itself:
I already configured my app to use Below I will write what can be improved in fiber by adding a new option. But nevertheless, even if you start working with the body at the very beginning, some kind of edge case may still arise and It’s not entirely clear how to make it not crash in a panic so that recover works, because About PR with immutability for body:
And it is clear that there is another option
And then work with the already copied body. Also can work with copied body in additional context with custom
https://go.dev/play/p/NBFC_NLT3lV And also will do modify
Thanks. |
@asyslinux thanks for the analysis an additional setting is not necessary, only the query of the setting in the respective methods and the making of the copy at the end shortly before the return |
@asyslinux could you support us with a PR ? |
@ReneWerner87, yes, I will make a PR on next week, with addons to documentation(wiki) and I need a little time to test edge cases when a nil pointer error occurs in fasthttp, since for some reason |
@ReneWerner87 I don't quite understand which code option is preferable:
Which method is better in the case of the body? |
@ReneWerner87, I checked what will happen in both cases, firstly, Go by default does not inline the Even such a simple example with the I prefer 1 too. Let's wait what others say. |
@asyslinux you are welcome to start, further comments may be made in the review, otherwise it looks ok for solution 1 |
@ReneWerner87, please review PR when you have a time. I will add a little later the results of practice tests that cause panic(I'm still dealing with this problem) and an example of a high-performance application for receiving and processing a large number of POST requests. |
Bug Description
Immutable: true
Application receives 1000rps+ requests with POST data. In 6 months of continuous operation of the application, I encountered these two panics only 2 times. I do not understand, how can be nil pointers with immutable true...I don't even know what to guess.
It seems to me that this can only happen if some kind of premature interruption of a request in fasthttp and there comes a moment when my code in the application is already running and was not interrupted for some reason and in a rare case a call to the nil pointer occurs. But this is just a guess.
Fiber settings:
This panics from 1 server:
Requests with 499 HTTP CODE (client closed request) from nginx:
This panics from 2 server:
Requests with 499 HTTP CODE (client closed request) from nginx:
How to Reproduce
I do not know how to reproduce, I guess this sometimes happens with 499 (client closed request).
Maybe I missed something when writing an application to process POST requests. Maybe need to write code using OnClose? In case the client closes the connection ahead of time. But just how to do this, I don’t know at what point in time the client will break the connection, in my code anywhere there can be access to headers, body, request parameters, this is not an atomic operation.
[email protected]/ctx.go:376
fiber/ctx.go
Line 376 in af39998
[email protected]/ctx.go:382
fiber/ctx.go
Line 382 in af39998
[email protected]/ctx.go:560
fiber/ctx.go
Line 560 in af39998
Expected Behavior
Usually everything works without problems, serves a lot of requests, never panics for months.
Below in my code you can see that panic occurs not only when the body is processed for the first time, but also when it is processed again, also on line 1040 of my code. That is, imagine that there was a body in fasthttp and there was no panic on line 1033, and then it disappeared somewhere in fasthttp and a panic happened on line 1040. Yes, and the first processing of a POST body request can also cause panic right away.
More precisely, the entire fasthttp.Request disappears somewhere or does not have time to be copied when client closing connection, but my code is executed and essentially accesses fasthttp.Request via fiber. But interesting case, when on line 1033 fasthttp.Request have data, but little later on line 1040 fasthttp.Request is no have data, how data on request with immutability enabled, which should have already been copied and successufully accessed on line 1033, suddenly disappeared somewhere and on line 1040 i get a nil pointer.
Fiber Version
v2.50.0 and below
Code Snippet (optional)
When processing POST requests, I made an additional context with a timeout and placed the request processing in the go func(), this was done just in case, so that if anything happens, the processing of the POST request is guaranteed to complete. But I can definitely say that if panic occurs, the timeout of 3 seconds does not occur.
Partly my code:
Checklist:
The text was updated successfully, but these errors were encountered: