Opened 2 years ago

Closed 2 years ago

#23381 closed Bug (fixed)

Context manager should determine their initial state at __enter__, not __init__

Reported by: Thomas Chaumeny Owned by: nobody
Component: Utilities Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Some context manager use init to determine their initial state and exit to restore that state. The correct way to use context manager is to determine the initial state within the enter method. This is especially dangerous when using ContextDecorator where initialization time can be different from "entering" time.

Flawed context managers are django.utils.translation.override (which leads to a bug following the use of ContextDecorator) and django.utils.timezone.override. The pull request below fix them both and updates a test to demonstrate the failure with django.utils.translation.override.

Change History (4)

comment:1 Changed 2 years ago by Thomas Chaumeny

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 2 years ago by Aymeric Augustin

Component: UncategorizedUtilities
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Good catch. See my comments on the pull request. If you're not sure how to write the tests, let me know, I can take care of that part.

comment:3 Changed 2 years ago by Aymeric Augustin

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Updated PR looks good.

comment:4 Changed 2 years ago by Simon Charette <charette.s@…>

Resolution: fixed
Status: newclosed

In efcbf3e095dce3491eb52774978afe3f7bbdf217:

Fixed #23381 -- Context manager restored state should be determined in enter

Note: See TracTickets for help on using tickets.
Back to Top