algorand / algorand/go-algorand

tealdbg: Enforce critical regions for concurrent data access

未关闭
#3,850 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
new-feature-request Team Scytale
主要语言
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 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。