Code

Opened 6 years ago

Closed 6 years ago

#7475 closed (fixed)

Race Condition in ModelChoiceIterator (affects ModelChoiceField and ModelMultipleChoiceField)

Reported by: esaj Owned by: nobody
Component: Forms Version: master
Severity: Keywords: race condition threading ModelChoiceIterator generator ValueError
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation:
Needs tests: Patch needs improvement:
Easy pickings: UI/UX:

Description

I have discovered a race condition in ModelChoiceIterator: when ModelChoiceField is initialised with a queryset, this same queryset is kept across multiple requests (due to the fact that form fields are generally only initialised once as class members). As a result, there is a race condition when multiple requests are made concurrently, and more than one request tries to iterate over the same queryset. This generally appears in the form of a ValueError: generator already executing occurring.

The Select widget also seems to keep the same ModelChoiceIterator instance in its choices attribute across multiple requests. Under high load, this ModelChoiceIterator may be consumed by multiple threads simultaneously. However, as this queryset instance is the same for these multiple threads, it causes a ValueError: generator already executing error when the second or third thread attempts to consume it.

The patch is extremely simple for this: just call .all() on the queryset every time the ModelChoiceIterator is consumed. I've tested this under high load and it works perfectly.

Attachments (3)

models.py.diff (797 bytes) - added by esaj 6 years ago.
Simple patch that fixes the race condition.
models.2.py.diff (1.4 KB) - added by esaj 6 years ago.
Add support for cache_choices option.
models.3.py.diff (1.4 KB) - added by esaj 6 years ago.
Slightler better patch than the previous one.

Download all attachments as: .zip

Change History (4)

Changed 6 years ago by esaj

Simple patch that fixes the race condition.

Changed 6 years ago by esaj

Add support for cache_choices option.

Changed 6 years ago by esaj

Slightler better patch than the previous one.

comment:1 Changed 6 years ago by jacob

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

(In [7710]) Fixed #7475: fixed a possible race condition in ModelChoiceIterator. Thanks, esaj.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.