algorand / algorand/go-algorand
tealdbg: Enforce critical regions for concurrent data access
- 主要语言
- Go
- 星标
- 1.4k
- 派生
- 537
- 平均合并
- 1 天 7 小时
- 30 天内合并 PR
- 18
描述
## Problem
As discussed in https://github.com/algorand/go-algorand/pull/3653#discussion_r834676699, `Debugger.Update` updates shared state _without_ acquiring a lock for the entire sequence of updates. Consequently, it's possible state will be partially updated and unexpectedly read/modified by another goroutine.
In practice, the chance for partial state update _seems_ low because tealdbg interaction happens slowly. A person stepping through a program is _probably_ moving slowly enough that the problem won't occur. However, future tealdbg changes are liable to make the problem more evident.
## Solution
* Lock reads/updates transactionally instead of per operation as is currently done. The entire sequence of shared state reads/writes must be guarded within a critical region.
* Consider if the locking mechanism provides re-entrant capabilities. Depending on the implementation, a reentrant lock is necessary.
* Remove atomics from `session.Line` in favor of critical regions like all other shared state.
* It's worth double checking _if_ a rationale exists for using atomics. From https://github.com/algorand/go-algorand/pull/3653#discussion_r834676699 and related verbals, no rationale was presented.
## Dependencies
N/A
## Urgency
TBD
贡献指南
评估
这个 Issue 还没有评估数据。