Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Unreleased

* Support Python 3.13.

* Fix a bug introduced in version 6.0.0 where ``Range`` requests could lead to database connection errors in other requests.

Thanks to Per Myren for the detailed investigation and fix in `PR #612 <https://github.com/evansd/whitenoise/pull/612>`__.

6.7.0 (2024-06-19)
------------------

Expand Down
1 change: 1 addition & 0 deletions src/whitenoise/responders.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def read(self, size=-1):
return data

def close(self):
super().close()
self.fileobj.close()


Expand Down
42 changes: 42 additions & 0 deletions tests/test_responders.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from __future__ import annotations

from io import BytesIO

from django.test import SimpleTestCase

from whitenoise.responders import SlicedFile


class SlicedFileTests(SimpleTestCase):
def test_close_does_not_rerun_on_del(self):
"""
Regression test for the subtle close() behaviour of SlicedFile that
could lead to database connection errors.

https://github.com/evansd/whitenoise/pull/612
"""
file = BytesIO(b"1234567890")
sliced_file = SlicedFile(file, 1, 2)

# Emulate how Django patches the file object's close() method to be
# response.close() and count the calls.
# https://github.com/django/django/blob/345a6652e6a15febbf4f68351dcea5dd674ea324/django/core/handlers/wsgi.py#L137-L140
calls = 0

file_close = sliced_file.close

def closer():
nonlocal calls, file_close
calls += 1
if file_close is not None:
file_close()
file_close = None

sliced_file.close = closer

sliced_file.close()
assert calls == 1

# Deleting the sliced file should not call close again.
del sliced_file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably take a weakref.ref(sliced_file) and wait for it to be dead here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the advantage of that approach, can you explain? The del approach triggers __del__ and failed before the fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test will pass if anyone accidentally makes a reference cycle in SlicedFile

assert calls == 1