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: tchaumeny 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


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 tchaumeny

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by aaugustin

  • Component changed from Uncategorized to Utilities
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

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 aaugustin

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

Updated PR looks good.

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

  • Resolution set to fixed
  • Status changed from new to closed

In efcbf3e095dce3491eb52774978afe3f7bbdf217:

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

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