python / python/cpython

Race between UserDict.__getitem__ and __delitem__ can result in __missing_ not being called when item is missing.

オープン
#156,544 コメント 5 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

stdlib topic-free-threading type-bug
主要言語
Python
スター
77.2k
フォーク
35.9k
PR マージ指標
PR 指標を取得中

説明

https://github.com/python/cpython/blob/71cb9e8b64c4a89971955d3ce8ff393f283aeace/Lib/collections/__init__.py#L1189

I believe there is a race if __getitem__ sees the key exists in self.data and the GIL is released before getting the item. If another thread then calls __delitem__ which removes the item from self.data, when __getitem__ resumes it will attempt to get and return the item which no longer exists resulting in KeyError being raised out of __getitem__. If the class has __missing__ defined it should be called if the item is missing, but that won't occur in this sequence of events.
This is a theoretical bug report based solely on code inspection prompted by a DPO discussion about a different race the GIL does protect against (https://discuss.python.org/t/pep-805-safe-parallel-python/108670/49).

The only concern is that __missing__ will not be called, not that the item is initially seen then not seen.
I have not done a thorough review yet of the rest of the UserDict methods that do a containment check followed by lookup, but my cursory inspection they look to delegate to __getitem__. I am a bit concerned that get() does not call __missing__ if the initial check does not see the item but will if it sees the item and this bug is fixed as proposed below will result in __missing__ being called. This should be consistent.
https://github.com/python/cpython/pull/17910 seems related and I will look into the details of that fix to ensure this fix is consistent (if I end up working on this issue).

I offer a strawman proposal for fixing this by changing __getitem__ to use a single inspection of self.data with something like:
item = self.data.get(key, MISSING) if item is not MISSING: return item ...
Where MISSING is an internal object guaranteed to not be a key in self.data (adding it if something suitable doesn't already exist).

I volunteer to work on this issue. To point me in the right direction are there existing tests (either in collections or elsewhere) that force GIL timing races I can look at as an example of how to write a test to verify this issue is fixed? The challenge is reliably interleaving the initial containment check, then __delitem__, then the subscript access. I'm not sure if this level of testing is feasible based on previous experience with similar issues...the fix usually invalidates the test steps to interleave things properly by removing the ability for them to be interleaved. Any guidance on this would be very appreciated if my offer to work on this is taken up.

Linked PRs
  • gh-156621
  • gh-157287

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

Lib/collections/init.py の 1189 行目付近から始め、報告で説明されている UserDict の検索および削除の動作を調査してください。リンクされている PR gh-156621 と gh-157287 を確認して現在の方向性を把握し、その後、既存の collections テストで missing と関連するインターリービングがカバーされているか確認してください。報告された欠落項目の動作が get() と一貫する形で解決され、適切なテストでカバーされていれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
python
領域
backend
issue の種類
バグ
難易度
4/5
見積もり時間
3〜5日
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。