Skip to content
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

Define the architecture of the HTTP message ownership #2296

Open
krizhanovsky opened this issue Nov 28, 2024 · 0 comments
Open

Define the architecture of the HTTP message ownership #2296

krizhanovsky opened this issue Nov 28, 2024 · 0 comments
Labels
enhancement h2 http/1.1 question Questions and support tasks
Milestone

Comments

@krizhanovsky
Copy link
Contributor

Motivation

The task arose from #2288 (review) :

The patch looks good, but the fix, or I'd say the current architecture, is very fragile. tfw_h2_stream_unlink_nolock() and tfw_h2_stream_unlink_lock() are called from a dozen functions with various HTTP/2 streams logic and there is no direct linkage between these functions and the TFW_HTTP_B_FULLY_PARSED flag. Moreover, the flag is set by HTTP parser and there is no immediate pairing of the request with response. I.e. there is no guarantee (or at least the guarantee is unclear and/or can be easily broken in future) that we can't fully process an HTTP request and immediately call tfw_h2_stream_unlink_nolock() (probably before even before getting a response!). In other words it seems there is no guarantee that we don't have a memory leak.

At the moment we have spaghetti-like code: there are many place (and even different .c files aka 'program modules'), which in charge of freeing HTTP messages. We don't have a clear architecture of who owns a message. The same is probably about other objects. This makes code changes hard (a trivial feature or bugfix may change many files), produces memory leaks and corruptions.

Scope

Need to revise the architecture and refactor the code. Maybe linked with #687 since we also planned to split http.c code and make it more scalable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement h2 http/1.1 question Questions and support tasks
Projects
None yet
Development

No branches or pull requests

1 participant