#30565: HttpResponseBase.close not called when using FileResponse / WSGI "file
wrapper" object
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Uncategorized | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: HttpResponse, | Triage Stage:
streaming, StreamingHttpResponse | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
> You implemented some kind of custom FileResponse and for some reason the
file object you pass as the streaming_content argument is not being
closed. Is this correct so far?
No, that was in my original post before I had developed a correct
understanding of the issue. After diagnosing the issue, I updated the
ticket's title with the new understanding ("HttpResponseBase.close not
called when using FileResponse / WSGI "file wrapper" object") and changed
the first sentence of my original post to read, "See comment #6 further
down for the correct, updated description for this ticket."
What I think should be addressed is that either the code comment above
`HttpResponseBase.close()` should be clarified to say that it's not called
if "file_wrapper" is used, or else `FileResponse._set_streaming_content()`
should wrap the file-like object's `close()` method so that when the WSGI
server calls `close()` on the file-like object, it also calls `close()` on
the corresponding `HttpResponseBase` object. (I provided a proof-of-
concept for how that can be done in
[https://code.djangoproject.com/timeline?from=2019-06-15T03%3A11%3A43-05%3A00&precision=second
last comment above].)
Here are a few reasons I think the latter should be done. First,
`HttpResponseBase.close()` raises the `request_finished` signal, which
Django's documentation
[https://docs.djangoproject.com/en/2.2/ref/signals/#django.core.signals.request_finished
says] is "sent when Django finishes delivering an HTTP response to the
client." There are a number of valid reasons someone might want to act on
the end of the request. Currently, people won't be getting that signal
simply because "file_wrapper" happens to be used, and they might not know
they aren't. "file_wrapper" is just an optimization, so it doesn't seem
like it should be affecting Django's behavior otherwise.
The second reason is that it's not obvious for the developer to know in
advance if "file_wrapper" will be used because a number of conditions need
to met and those conditions even depend on the deployment setting (e.g.
whether the environment contains `wsgi.file_wrapper`). So it's hard for
the developer to know whether they can rely on the `request_finished`
signal for a given request.
Third, Django's code even seems to expect that `HttpResponseBase.close()`
will be called in the "file_wrapper" case because in its "file_wrapper"
code path `FileResponse._set_streaming_content()` adds the file-like
object to `self._closable_objects`, which is only used / needed in the
`HttpResponseBase.close()` case.
An alternative idea for a code change would be not to wrap the file-like
object's `close()` so that it calls `HttpResponseBase.close()`, but
instead to (1) update the code comment to clarify that it's not called
when "file_wrapper" is used, and (2) wrap the file-like object's `close()`
so that it sends the `request_finished` signal. That way developers will
be able to rely upload the `request_finished` signal in both situations.
--
Ticket URL: <https://code.djangoproject.com/ticket/30565#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.a0683bc33a33b170c9c0774a2b66afd7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
