Skip to content

Fix: remove redundant lock(this) in ReadFromDateAndRegion#23

Open
HashidaTKS wants to merge 1 commit into
masterfrom
fix/redundant-lock
Open

Fix: remove redundant lock(this) in ReadFromDateAndRegion#23
HashidaTKS wants to merge 1 commit into
masterfrom
fix/redundant-lock

Conversation

@HashidaTKS

Copy link
Copy Markdown
Owner

問題点

HoldingInformationRepository.ReadFromDateAndRegion() で外側に lock(this) を設定しているが、内部で呼ぶ ReadAll()BaseRepository 内で lock(this) を持つ。

C# の lock はリエントラントなのでデッドロックは発生しないが、二重ロックは設計の意図が不明瞭で、将来の変更者に混乱を与える。

改善内容

  • ReadFromDateAndRegion の外側の lock(this) を削除
  • ReadAll() の内部ロックで排他制御が担保される

Test plan

  • ReadFromDateAndRegion が正しい結果を返すことを確認
  • 排他制御が失われないことを確認(ReadAll 内のロックが機能している)

🤖 Generated with Claude Code

ReadAll() already acquires lock(this) internally in BaseRepository,
making the outer lock(this) in ReadFromDateAndRegion a redundant reentrant
lock. C# lock is reentrant so it doesn't deadlock, but the duplicate
locking is misleading and suggests the locking strategy is misunderstood.
Remove the outer lock as ReadAll() handles synchronization.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant