Skip to content

common: fix task leak in timer #4831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025
Merged

common: fix task leak in timer #4831

merged 1 commit into from
Jun 23, 2025

Conversation

isluckys
Copy link
Contributor

signal包里面SetTimeout方法并发时可能会出现task close以后执行start导致泄露

signal包里面SetTimeout方法并发时可能会出现task close以后执行start导致泄露
@Fangliding Fangliding linked an issue Jun 20, 2025 that may be closed by this pull request
4 tasks
@Fangliding
Copy link
Member

好像确实有极低的概率泄露出去一个Periodic 你是在干什么的时候遇到的?

@yuhan6665
Copy link
Member

抱歉没看出来泄露的原因。。能否详细描述或者加一个 unit test?

@Fangliding
Copy link
Member

Fangliding commented Jun 23, 2025

抱歉没看出来泄露的原因。。能否详细描述或者加一个 unit test?

t.Unlock() 让出锁之后 checkTask 就可以在外面通过 t.checkTask 访问了 这时候如果有一个进程用 SetTimeout(0) 触发 finish() 会停掉 checkTask 但是后面又 common.Must(checkTask.Start()) 把这个checkTask 又拉起来了 又没人来负责关闭它 就会一直运行运行
但是实际上触发的概率非常非常低 用 go test -race 可能都触发不了 必须在Unlock后手动用 runtime.Gosched() 触发才可以稳定复现 所以我问怎么遇到的(

@isluckys
Copy link
Contributor Author

抱歉没看出来泄露的原因。。能否详细描述或者加一个 unit test?

t.Unlock() 让出锁之后 checkTask 就可以在外面通过 t.checkTask 访问了 这时候如果有一个进程用 SetTimeout(0) 触发 finish() 会停掉 checkTask 但是后面又 common.Must(checkTask.Start()) 把这个checkTask 又拉起来了 又没人来负责关闭它 就会一直运行运行 但是实际上触发的概率非常非常低 用 go test -race 可能都触发不了 必须在Unlock后手动用 runtime.Gosched() 触发才可以稳定复现 所以我问怎么遇到的(

在连接连入立即关闭时也会出现这种问题。Proxy 那边的 request 和 response 都有 SetTimeout,req 或者 resp 的锁解除的一瞬间,另一个会直接到关闭流程,但是 checkTask.Start() 这时可能还没有执行导致泄露。

@isluckys
Copy link
Contributor Author

抱歉没看出来泄露的原因。。能否详细描述或者加一个 unit test?

t.Unlock() 让出锁之后 checkTask 就可以在外面通过 t.checkTask 访问了 这时候如果有一个进程用 SetTimeout(0) 触发 finish() 会停掉 checkTask 但是后面又 common.Must(checkTask.Start()) 把这个checkTask 又拉起来了 又没人来负责关闭它 就会一直运行运行 但是实际上触发的概率非常非常低 用 go test -race 可能都触发不了 必须在Unlock后手动用 runtime.Gosched() 触发才可以稳定复现 所以我问怎么遇到的(

关于如何发现这个,是因为这边有非常多高并发。运行久了以后gc会占用比较高,pprof的goroutine是看不到相关的,在下载trace分析以后会非常明显。没有看出来很正常因为我也查了一个星期多才定位并且分析出来原因。

@yuhan6665 yuhan6665 changed the title 修复task泄露 common: fix task leak in timer Jun 23, 2025
@yuhan6665 yuhan6665 merged commit cb1afb3 into XTLS:main Jun 23, 2025
39 checks passed
@yuhan6665
Copy link
Member

懂了 大佬感谢修复!

patterniha pushed a commit to patterniha/Xray-core that referenced this pull request Jun 28, 2025
signal包里面SetTimeout方法并发时可能会出现task close以后执行start导致泄露
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

signal包泄露定时任务
3 participants