スレッドセーフでないコードをレビューでどう発見するか
はじめに
Pythonはthreadingモジュールや並列タスクの活用によって、複数の処理を同時に走らせることができます。
しかし、スレッドセーフでないコードが混入したままレビューをすり抜けるケースは、実務上しばしば見られます。特に初学者が触れたコードや、小規模実装が発展してきた構成では、意図せぬデータ競合やミューテーションの衝突が発生しやすくなります。
本記事では、レビューアーとして「スレッドセーフでない構造をどう発見し、どう指摘するか」をテーマに、具体的な判断軸を提供します。
スレッド非安全な構造:典型的なコード例
グローバルキャッシュの書き換え
cache = {}
def get_or_compute(key, compute_fn):
if key not in cache:
cache[key] = compute_fn()
return cache[key]このコードは複数スレッドが同時に同じkeyにアクセスする場合、compute_fn()が重複実行される可能性があります。
レビュー指摘コメント
@Reviewer: グローバル辞書`cache`に対して同時書き込みが発生する構造はスレッド非安全です。
排他制御(Lock)やスレッドローカル変数、または構造分離による対応を検討してください。スレッドセーフとは
スレッドセーフとは、「複数のスレッドから同時にアクセスされても、状態が破壊されず、常に正しい結果を返す設計」であることを意味します。
共有リソースに対する排他制御(mutex、ロック)や、ステートレス設計、スレッドローカルストレージの活用などが該当します。
スレッド非安全構造を見抜くレビュー観点
- グローバル変数・モジュールスコープ変数への書き込み
- インスタンス変数の状態を更新する関数が複数スレッドから呼ばれている
- イテレータやジェネレータを外部スレッドと共有している
- 非同期処理との混在(
async+threading) - デフォルト引数にミュータブル型を使っている
ミュータブルデフォルト引数(危険)
def append_to_log(entry, log=[]):
log.append(entry)
return logデフォルト引数のレビュー
@Reviewer: `log=[]` は関数定義時に共有されるため、スレッド間で状態が競合する可能性があります。
`None`で初期化し、内部で生成するよう変更すべきです。スレッド非安全なキャッシュアクセスの構造図
この図では、ThreadAとThreadBが同時にSharedCacheへアクセスする状況を示しています。
同期機構がなければ、最終的な状態は不定です。
排他制御による改善案
Lockによる制御
import threading
cache = {}
lock = threading.Lock()
def get_or_compute(key, compute_fn):
with lock:
if key not in cache:
cache[key] = compute_fn()
return cache[key]レビューコメント:同期の明示化
@Reviewer: `Lock`によって`cache`の読み書きが排他制御され、安全性が確保されました。
ただし、ロックの粒度や範囲には注意が必要です。レビュー時に確認すべき排他制御の観点
- Lockの取得・解放のスコープが最小限に抑えられているか?
- 共有リソース単位でロックが分離されているか?
- デッドロックの可能性(複数Lockの組み合わせ)を排除しているか?
- 単にスレッドセーフにしたいだけであればQueueやThreadSafe構造を使えないか?
スレッドローカルストレージの活用
threading.localを使った構造
import threading
local_data = threading.local()
def set_context(value):
local_data.context = value
def get_context():
return getattr(local_data, 'context', None)このように、スレッドごとに独立した状態を管理することで、状態の混線を防ぐ設計も可能です。
レビュー補足
@Reviewer: `threading.local()` による状態管理はスレッドごとの分離が保証されており、排他制御が不要になります。
ただし、明示的にスレッド設計をしている場合に限定すべきです。スレッド安全な標準ライブラリの活用
| 構造 | スレッドセーフ性 | コメント |
|---|---|---|
queue.Queue |
◎ | 標準で排他制御あり |
dict, list |
× | 明示的にLock必要 |
loggingモジュール |
◎ | 内部でLock使用 |
sqlite3 |
△(設定次第) | check_same_thread=Falseに注意 |
collections.deque |
△ | append/popはスレッドセーフ |
結論:スレッド安全性のレビューは「構造を追う技術」
スレッドセーフでないコードは意図的に作られるのではなく、設計が意識されないまま生まれるケースが大半です。
レビューアーとして確認すべきは以下の通りです:
- グローバル変数・インスタンス変数に対して同時アクセスされる可能性がある構造か?
- 非同期や並列処理が入り込む箇所で、明示的な制御構造が存在しているか?
- 排他制御が「されていること」よりも、「設計意図として必要だったかどうか」が明示されているか?
コードが並列化される以上、意図が明示されていない共有はバグの温床です。
スレッドセーフ性は、設計と構造の読み解きによってレビューで見抜くことができます。
