已合并
fix: add PyObject_ClearWeakRefs in NPU Stream/Event tp_dealloc #41596
21xiu创建于 7月14日
fix: add PyObject_ClearWeakRefs in NPU Stream/Event tp_dealloc #41596
已合并
Pull Request已成功合入, 合并人@ascend-robot
(感谢 21xiu 的贡献)atomgit-bot
7月14日 评论:
7月14日 评论:
变更摘要
此 PR 修复了 NPU Stream 和 Event 对象的 tp_dealloc 中缺少弱引用清理的问题。通过在析构函数中调用 PyObject_ClearWeakRefs,确保在对象内存被释放前清除所有弱引用,避免弱引用回调访问已释放内存。同时,Stream 的析构函数还增加了 Py_CLEAR(self->context) 以正确释放 context 字段,防止内存泄漏。
主要改动
torch_npu/csrc/npu/Event.cpp: 在THNPEvent_dealloc函数中,于调用tp_free之前添加PyObject_ClearWeakRefs((PyObject*)self),确保 Event 对象的弱引用在析构时被清理。torch_npu/csrc/npu/Stream.cpp: 在THNPStream_dealloc函数中,添加PyObject_ClearWeakRefs((PyObject*)self)清理弱引用,并添加Py_CLEAR(self->context)释放context字段持有的引用。


ascend-robot
7月14日 评论:
7月14日 评论:
atomgit-bot
7月14日 评论:
7月14日 评论:
代码审查
审查总结
对 2 个变更文件完成审查,每个文件均已逐行分析:
torch_npu/csrc/npu/Event.cpp:发现 1 个问题(P3)torch_npu/csrc/npu/Stream.cpp:发现 1 个问题(P3)
统计
| 优先级 | 数量 |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 0 |
| P3 | 2 |
整体风险判断
低风险。本次 PR 在两个 tp_dealloc 函数中新增了 PyObject_ClearWeakRefs 调用,并在 THNPStream_dealloc 中修复了 context 字段的潜在泄漏(Py_CLEAR),这些都是正确的改进。唯一的问题是 PyObject_ClearWeakRefs 的调用顺序:当前在 C++ 析构函数(~NPUEvent() / ~NPUStream())之后调用,而 CPython 文档推荐在对象字段失效之前调用。在大多数实际场景中不会触发问题(因为弱引用回调在 Stream/Event 类型上极为罕见),但建议调整为推荐顺序以消除理论上的未定义行为风险。
⚠️ 已识别出整体风险,但无法提取行内评论,请参考整体评估。


7月14日 添加了label:ascend-cla/yes
此处折叠了90条消息 查看更多
7月15日 添加了label:approved
7月16日 添加了label:lgtm
7月16日 合入了pull request
ascend-robot
7月16日 评论:
7月16日 评论:
流水线 pytorch_gitcode_PR_multiVersion#12663 [ commitID:a6342440 ] 已完成


【合入来源】
【修改方案】
PyTorch 基础类 THPStream / THPEvent 中增加了 weakref 支持(添加了 weakreflist 字段),但 NPU 后端的 tp_dealloc 覆盖函数未同步更新。
CPython 对静态 PyType 不会自动链式调用父类 tp_dealloc,因此 THNPEvent_dealloc 和 THNPStream_dealloc 一直缺少:
【资料变更】
不涉及
【接口变更】
不涉及
【功能验证】
pytest test/dynamo/test_stream.py -k test_npu_stream_event_weakref_callback


修改前:
修改后:
【CheckList】