provide access to raw hyper::Request via an extractor
#555
Comments
This was referenced Jan 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we provide every request handler with an
rqctx: Arc<RequestContext>, which containsrqctx.request: Arc<Mutex<hyper::Request>>.(Why? Access to the raw
hyper::Requestis useful as an escape hatch for stuff that Dropshot currently doesn't support. The two use cases I know of are websockets (now supported) and streaming bodies (work in progress). Although we expect these both to be supported in the not-too-distant future, this escape hatch remains useful.)But we've always known that this way of exposing it is janky. There's only one thing with access to the request, so it's annoying that it's wrapped in an Arc+Mutex. Worse: under #542 and related PRs, we're considering support for streaming request bodies. These also need access to the
hyper::Request(well, specifically, the body, which can only be gotten by consuming the request). By allowing access to all handlers via the Arc+Mutex, it's possible to wind up with a deadlock, which means imposing implicit locking rules on consumers that we have no way to enforce.Instead, let's remove the raw
hyper::Requestfrom theRequestContextand create a new extractor for getting it. This way, if we can also ensure that no endpoint has two extractors that want access to the request (#556), there's no Arc+Mutex, so less awkwardness and no locking issues.Note that there have been two attempts at this:
&mut Request<Body>#106 kind of ran out of steam after I dropped the ball.I've got a prototype of shared/exclusive extractors in #556 that's looking promising. With that, I think we can make
TypedBody,UntypedBody, and a newRawRequestintoExclusiveExtractors, which means you can only have one of these as an argument to your endpoint function.The text was updated successfully, but these errors were encountered: