Skip to content

Commit dd7f7f0

Browse files
committed
Fix: [Server][Utils/TwitterGraphQLAPI] やっぱりロック不要じゃね?ということに気づいたので削除
1 parent 1618512 commit dd7f7f0

File tree

1 file changed

+23
-22
lines changed

1 file changed

+23
-22
lines changed

server/app/utils/TwitterGraphQLAPI.py

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ class TwitterGraphQLAPI:
5959

6060
# Twitter アカウント ID ごとのシングルトンインスタンスを管理する辞書
6161
__instances: ClassVar[dict[int, TwitterGraphQLAPI]] = {}
62-
# __instances へのアクセスを保護するためのロック
63-
__instances_lock: ClassVar[asyncio.Lock] = asyncio.Lock()
6462

6563
# 必ず Twitter アカウント ID ごとに1つのインスタンスになるように (Singleton)
6664
def __new__(cls, twitter_account: TwitterAccount) -> TwitterGraphQLAPI:
@@ -77,6 +75,8 @@ def __new__(cls, twitter_account: TwitterAccount) -> TwitterGraphQLAPI:
7775
"""
7876

7977
# まだ同じ Twitter アカウント ID のインスタンスがないときだけ、インスタンスを生成する
78+
## __new__ は同期メソッドで await がないため、実行中は他のコルーチンに切り替わらない
79+
## そのため、__instances へのアクセスはアトミックであり、ロックは不要
8080
if twitter_account.id not in cls.__instances:
8181

8282
# 新しいインスタンスを作成する
@@ -148,26 +148,27 @@ async def removeInstance(cls, twitter_account_id: int) -> None:
148148
twitter_account_id (int): 削除する Twitter アカウントの ID
149149
"""
150150

151-
# __instances へのアクセスを保護するため、ロックで保護する
152-
async with cls.__instances_lock:
153-
if twitter_account_id in cls.__instances:
154-
instance = cls.__instances[twitter_account_id]
155-
# シャットダウンタスクをキャンセル
156-
## 複数の同時リクエスト完了時の競合状態を防ぐため、ロックで保護する
157-
async with instance._shutdown_task_lock:
158-
if instance._shutdown_task is not None:
159-
if not instance._shutdown_task.done():
160-
instance._shutdown_task.cancel()
161-
instance._shutdown_task = None
162-
# ヘッドレスブラウザをシャットダウン
163-
## browser.shutdown() が例外を投げた場合でもレジストリエントリは確実に削除する必要があるため、try/except で囲む
164-
if instance._browser is not None and instance._browser.is_setup_complete is True:
165-
try:
166-
await instance._browser.shutdown()
167-
except Exception as ex:
168-
logging.error(f'Failed to shutdown browser for Twitter account {twitter_account_id}:', exc_info=ex)
169-
# レジストリエントリを削除
170-
del cls.__instances[twitter_account_id]
151+
# __instances へのアクセスは await がないためアトミックであり、ロックは不要
152+
## __new__ は同期メソッドで await がないため、実行中は他のコルーチンに切り替わらない
153+
## removeInstance の __instances へのアクセス部分(チェック・取得・削除)も await がないため、その部分はアトミック
154+
if twitter_account_id in cls.__instances:
155+
instance = cls.__instances[twitter_account_id]
156+
# シャットダウンタスクをキャンセル
157+
## 複数の同時リクエスト完了時の競合状態を防ぐため、ロックで保護する
158+
async with instance._shutdown_task_lock:
159+
if instance._shutdown_task is not None:
160+
if not instance._shutdown_task.done():
161+
instance._shutdown_task.cancel()
162+
instance._shutdown_task = None
163+
# ヘッドレスブラウザをシャットダウン
164+
## browser.shutdown() が例外を投げた場合でもレジストリエントリは確実に削除する必要があるため、try/except で囲む
165+
if instance._browser is not None and instance._browser.is_setup_complete is True:
166+
try:
167+
await instance._browser.shutdown()
168+
except Exception as ex:
169+
logging.error(f'Failed to shutdown browser for Twitter account {twitter_account_id}:', exc_info=ex)
170+
# レジストリエントリを削除
171+
del cls.__instances[twitter_account_id]
171172

172173
async def __scheduleShutdownTask(self) -> None:
173174
"""

0 commit comments

Comments
 (0)