Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.
RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.

Bug 1897294

Summary: Race condition in OrderedDict.popitem()
Product: Red Hat Enterprise Linux 8 Reporter: Radomir Dopieralski <rdopiera>
Component: python3Assignee: Python Maintainers <python-maint>
Status: CLOSED WONTFIX QA Contact: RHEL CS Apps Subsystem QE <rhel-cs-apps-subsystem-qe>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 8.2CC: tkajinam, vstinner
Target Milestone: rcFlags: pm-rhel: mirror+
Target Release: 8.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-11-17 14:28:35 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1896614    

Description Radomir Dopieralski 2020-11-12 17:38:54 UTC
Description of problem:

The implementations of collections.OrderedDict.popitem() in both Python 2.x and Python 3.x contain a race condition, where they first check the dictionary for emptiness, and then proceed assuming it is not empty. If the dictionary becomes
empty in the intervening time due to code in another thread, an unexpected StopIteration exception is thrown.


Version-Release number of selected component (if applicable):


How reproducible:

This race condition seems to be very rare, but because we use OrderedDict in a caching system that gets called very fast a lot of times in a lot of threads, our
customers actually hit it in Horizon in rhbz#1896614

Steps to Reproduce:
1. Run a code that continuously adds and removes a single item of OrderedDict in a a background thread.
2. Call popitem on that dict, until you hit the race condition.
3. PROFIT

Actual results:
StopIterationn is thrown.

Expected results:
KeyError should be thrown.

Additional info:

Comment 1 Victor Stinner 2020-11-17 12:43:29 UTC
According to the customer ticket, Python 2.7 is being used. Before Python 3.5, you are correct that the OrderedDict.popitem() method was not thread safe.

To support Python 2.7, you must protect all (read/write) accesses to the OrderedDict by a lock in your application (Horizon).

Or you should attempt to upgrade to Python 3.5 or newer which is safe. RHEL8 provides Python 3.6 which is safe, for example.

Python collections.OrderedDict type cannot be modified to use a lock, it would likely introduce *new* deadlocks in multithreaded applications (like Horizon) which would not be prepared for that.

--

Before Python 3.5, collections.OrderedDict was implemented in pure Python. It was no thread safe since the type does not use a lock internally. Usually, Python builtin types are not thread safe by design, but only because they are implemented in C which makes most methods atomic thanks to the GIL. In practice, it's more complicated than that. If a C method calls arbitrary code, it can release the GIL and so is no longer thread safe.


Since Python 3.5, Python now also provides _collections.OrderedDict type which is implemented in C. collections/__init__.py:
---
class OrderedDict(dict):
    # Python implementation
    ...

try:
    from _collections import OrderedDict
    # C implementation overrides the Python implementation
except ImportError:
    # Leave the pure Python version in place.
    pass
---

The C implementation of the _collections.OrderedDict.popitem() method is protected by the GIL and so looks to be thread-safe.

Comment 2 Takashi Kajinami 2020-11-17 13:07:15 UTC
@Victor

You are correct and the deployment has python2.7 used, because it is the deployment with RHOSP13 on RHEL7.

Unfortunately I'm afraid that we can't update the python to 3.5.
There are no plans to update the base python for RHOSP13 so far, I don't think that update can happen
regarding the support phase of RHOSP13 and possible huge effect that update can cause.

So we should add some locking mechanism to Horizon, to downstream RHOSP13.
(And the two stable branches in upsteam, stable/rocky and stable/train, which support Python2, ideally)

We don't expect the same issue with RHOSP16.y which depends on python3.6 and RHEL8, according to your information.

Comment 3 Radomir Dopieralski 2020-11-17 14:28:35 UTC
I think that in OSP13 we can simply work around that issue by catching the unexpected exception. I'm closing this, since we are not going to be fixing Python.