algorand / algorand/go-algorand
tealdbg: Enforce critical regions for concurrent data access
- Ngôn ngữ chính
- Go
- Star
- 1.4k
- Fork
- 537
- Merge trung bình
- 1 ngày 6 giờ
- Pull request đã merge (30 ngày)
- 17
Mô tả
## 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
Hướng dẫn đóng góp
Đánh giá
Issue này chưa được đánh giá.