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

Async routes that have a body always exhibit thread-unsafety #2990

Closed
weissi opened this issue Mar 29, 2023 · 0 comments · Fixed by #2992
Closed

Async routes that have a body always exhibit thread-unsafety #2990

weissi opened this issue Mar 29, 2023 · 0 comments · Fixed by #2992

Comments

@weissi
Copy link
Contributor

weissi commented Mar 29, 2023

Vapor's Request is not thread safe which means that nothing must call request.<anything> unless you're guaranteed to be on request.eventLoop already (until #2951 is fixed).

But Vapor's own async code doesn't respect that rule.

For example this code here

_ = try await request.body.collect(max: max?.value ?? request.application.routes.defaultMaxBodySize.value).get()

needs to be

            if case .collect(let max) = body, request.body.data == nil {
                try await request.eventLoop.flatSubmit {
                    request.body.collect(max: max?.value ?? request.application.routes.defaultMaxBodySize.value)
                }.get()
            }

Obviously the above suggestion is just bandaid, Request has to become thread safe for it to be viable to use the async support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant