Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Infinite recursion issue with BodyProxy #564

Closed
Wardrop opened this Issue · 6 comments

3 participants

Tom Wardrop James Tucker Konstantin Haase
Tom Wardrop

Hi,

I've encountered an infinite recursion issue with BodyProxy. The issue is that by having #finish return self wrapped within a BodyProxy object, it allows for infinite recursion. The issue isn't the simplest to explain, so bear with me.

In my framework, Scorched, controllers can be nested to any arbitrary depth, so ControllerA may call ControllerB which calls ControllerC. There's no concept of a root/parent controller, so ControllerC for example has no idea it was called by ControllerB, hence every controller needs to comply with the Rack spec by accepting an env hash, and returning an array of three elements. So for each controller, I return the result of #finish after processing the request as to return the appropriate three element array.

The parent controller, like the sub controller, doesn't know it's called another controller. It just knows it's called a rack compliant object, which is all it needs to know. The parent controller takes the response, and merges it with it's own response object. In my framework, this response object is shared between controllers (it's stored in the env hash), so what happens is that when I assign the body object returned by a sub-controller to the response object of the parent controller, the body of my response object now contains a BodyProxy object that refers back to the response object, hence I get an infinite loop resulting in a stack overflow.

I can work around the issue by inspecting the body object that BodyProxy contains (checking if it's equal to self or not), but now my framework needs to be concerned with implementation details of Rack that may change in the future. Something else I can do to work-around the issue is to, instead of assigning the returned body object directly to the response object of the parent, I can iterate over the returned body object by calling #each, and assigning each value to a new array which I then assign as the body of the parent response object.

Sadly, I'm not sure what the best solution for this is. The whole problem only exists because #finish wraps self within a BodyProxy object. If BodyProxy only wrapped the body of the response object, and not the whole response object, you would avoid the issue.

I find it hard to recommend a solution because I'm not exactly sure what function BodyProxy serves or what problem it solves. I can only suspect that perhaps there's a better way that avoids creating the potential for infinite recursion. Could having BodyProxy wrap the actual response body rather than the response object be possible?

James Tucker
Owner

I don't think you want to really operate like this. You shouldn't need to be reassigning the body in this way.

BodyProxy is designed to help ensure that #close is always called based on the rules in SPEC. You can override this behavior without major issues, provided you conform to the close requirements in SPEC. The tricky part there is actually quite simple: if you override the body somewhere in a middleware chain, you must call close on the discarded body at end of your request processing - if you call it early, you could cause threading problems (assuming Rack::Lock and a non-thread safe application stack), if you don't call it, middleware like Rack::Lock will not release it's locks - resulting in deadlocks.

Tom Wardrop

Ok, so what I'll probably do then is override #finish on my Scorched::Response class which inherits from Rack::Response. I wanted to avoid this as I didn't understand what purpose BodyProxy served, but if you say it's safe, I'll do this.

Can I ask, why is the response object wrapped by BodyProxy and not the actual body of the response?

Konstantin Haase
Owner

The purpose of BodyProxy is to attach callbacks to an otherwise pretty much unknown body object. See CommonLogger and Lock for examples.

Tom Wardrop

@rkh That still doesn't quite answer why #finish wraps self in a BodyProxy. Lock and CommonLogger both create their own body proxies. Why does #finish, by default, wrap the response within a BodyProxy object? If some code requires BodyProxy for the sake of a callback, shouldn't they be creating their own BodyProxy?

James Tucker
Owner

Specifically to prevent infinite recursions: 5b251a9

@Wardrop can you provide a small code sample that represents your use case, so that I can better understand it? The text above isn't straightforward enough for me to understand the motivation behind your design decisions, or understand potential solutions.

James Tucker raggi added this to the Rack 1.6 milestone
James Tucker
Owner

Closing, waiting for feedback.

James Tucker raggi closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.