algorand / algorand/go-algorand

tealdbg: Enforce critical regions for concurrent data access

Đang mở
#3,850 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
new-feature-request Team Scytale
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

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.