aws / aws/sagemaker-python-sdk

_LocalPipeline Uses Class-Level Mutable State for Execution Registry

オープン
#5,572 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
2.3k
フォーク
1.3k
平均マージ
1日 22時間
マージ済み PR(30日)
35

説明

# Bug: `_LocalPipeline` Uses Class-Level Mutable State for Execution Registry

## PySDK Version

* [x] PySDK V3 (3.x)
* [ ] PySDK V2 (2.x)

## Describe the bug

The `_LocalPipeline` class defines `_executions` as a class-level mutable dictionary:

```python
class _LocalPipeline(object):
_executions = {}
```

This results in execution state being shared across all `_LocalPipeline` instances within the same Python process.

Because this registry is global to the class:

* Execution state is not isolated per pipeline instance
* Execution state is not isolated per `LocalPipelineSession`
* The structure is not thread-safe
* Execution objects accumulate without lifecycle management
* This can cause cross-pipeline contamination in multi-pipeline scenarios

This violates expected session scoping and encapsulation guarantees typically followed in AWS SDK design patterns.

---

## Why this matters

Local mode is commonly used for:

* Development workflows
* CI pipelines
* Unit/integration testing
* Multi-pipeline experimentation in notebooks

Shared mutable state introduces:

* Non-deterministic behavior
* Hard-to-debug test interference
* Memory growth in long-running processes
* Race conditions in concurrent execution environments

Even though this affects local mode only, isolation guarantees are still important for SDK correctness and developer trust.

---

## To reproduce

The following minimal example demonstrates that `_executions` is shared across instances:

```python
from sagemaker.mlops.local.pipeline_entities import _LocalPipeline

class DummyPipeline:
def __init__(self, name):
self.name = name
def definition(self):
return "{}"

pipeline1 = _LocalPipeline(DummyPipeline("pipeline1"))
pipeline2 = _LocalPipeline(DummyPipeline("pipeline2"))

pipeline1._executions["exec1"] = "execution1"
pipeline2._executions["exec2"] = "execution2"

print(pipeline1._executions)
print(pipeline2._executions)
print(pipeline1._executions is pipeline2._executions)
```

Output:

```
{'exec1': 'execution1', 'exec2': 'execution2'}
{'exec1': 'execution1', 'exec2': 'execution2'}
True
```

This confirms both instances reference the same dictionary object.

---

## Expected behavior

Each `_LocalPipeline` instance should maintain its own execution registry.

Example correction:

```python
class _LocalPipeline(object):

def __init__(self, ...):
...
self._executions = {}
```

This ensures:

* Proper execution isolation
* Predictable behavior across sessions
* Improved test determinism
* Elimination of unintended cross-instance state leakage

---

## System information

* **SageMaker Python SDK version**: main branch (sagemaker-mlops local module)
* **Python version**: 3.9+
* **CPU or GPU**: CPU
* **Custom Docker image**: No

---

## Proposed resolution

Move `_executions` from class scope to instance scope inside `_LocalPipeline.__init__`.

Optional enhancements for robustness:

* Consider execution cleanup strategy (TTL or explicit removal)
* Consider thread-safety if concurrent local execution is intended to be supported

---

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

sagemaker.mlops.local.pipeline_entities から始めて、_LocalPipeline クラスとその __init__ メソッドを調べます。issue の最小再現を実行して、現在の共有辞書の動作を確認します。完了の条件は、個別の _LocalPipeline インスタンスが実行レジストリを共有しなくなることです。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
aws, python
領域
machine-learning
issue の種類
バグ
難易度
2/5
見積もり時間
1〜3時間
活発さ
停滞
明瞭さ
明確に書かれている
初心者へのやさしさ
45/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。