Skip to content
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

refacor/Taskpool: 统一内外中断信号+修复bug #184

Merged
merged 4 commits into from
May 21, 2023

Conversation

longyue0521
Copy link
Collaborator

@longyue0521 longyue0521 commented May 20, 2023

  • 将shutdownCtx、shutdownNowCtx合并为一个interruptCtx
  • 重构方法、抽取方法、清理过期注释
  • 修复隐秘bug

@codecov
Copy link

codecov bot commented May 20, 2023

Codecov Report

Merging #184 (f860c5f) into dev (a120775) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #184      +/-   ##
==========================================
+ Coverage   95.85%   95.95%   +0.09%     
==========================================
  Files          44       44              
  Lines        2415     2396      -19     
==========================================
- Hits         2315     2299      -16     
+ Misses         79       77       -2     
+ Partials       21       20       -1     
Impacted Files Coverage Δ
pool/task_pool.go 99.12% <100.00%> (-0.05%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@longyue0521 longyue0521 changed the title refacor/Taskpool: 统一内外中断信号 refacor/Taskpool: 统一内外中断信号(WIP) May 20, 2023
2. 抽取numOfGoThatCanBeCreate方法,增强可读性
3. 对goroutine方法中注释进行整理,并抽取noTasksToExecute变量——增强可读性表明意图
2. 将gorutine方法中的time.NewTimer(1)改回为time.NewTimer(0)——延长时间不起作用
3. 修复隐秘bug
@longyue0521
Copy link
Collaborator Author

longyue0521 commented May 20, 2023

@flycash

当我在pool包内执行命令

 go test -run=Test -failfast -timeout=0 -count=1000 -v > a.out

会被卡死(因为设置了-timeout=0不会自动超时),经过排查发现:

atomic.AddInt32(&b.numGoRunningTasks, 1)
if !ok {
    // 1. G1执行atomic.LoadInt32(&b.state) == stateClosing为true后时间片用完,此时b.numGoRunningTasks==1
    // 2. G2先将b.numGoRunningTasks改为2,再进入这里因if判断条件为false,G2在下方2.1“else case”处用完时间片
    //      此时b.numGoRunningTasks == 2
    // 3. G1再次获得时间片,发现atomic.CompareAndSwapInt32(&b.numGoRunningTasks, 1, 0)为false
   // 从而导致atomic.CompareAndSwapInt32(&b.state, stateClosing, stateStopped)和b.shutdownCancel()永远不会被执行
   // 也导致了外部调用者的无限等待!!!!
    if atomic.LoadInt32(&b.state) == stateClosing && atomic.CompareAndSwapInt32(&b.numGoRunningTasks, 1, 0) {
	if atomic.CompareAndSwapInt32(&b.state, stateClosing, stateStopped) {
	    // 显示通知外部调用者
	    b.shutdownCancel()
        }
	b.decreaseTotalGo(1)
	return
    }

    // 2.1 "else case" G2在此处时间片用完,此时b.numGoRunningTasks == 2
    atomic.AddInt32(&b.numGoRunningTasks, -1)
    b.decreaseTotalGo(1)
    return
}

修复后使用命令

 go test -run=Test -failfast -timeout=0 -count=10000 -v > a.out

所有测试也能PASS,就是耗时很长

PASS
ok  	github.com/ecodeclub/ekit/pool	3439.027s

@longyue0521 longyue0521 changed the title refacor/Taskpool: 统一内外中断信号(WIP) refacor/Taskpool: 统一内外中断信号+修复bug May 20, 2023
@longyue0521 longyue0521 requested a review from flycash May 20, 2023 13:36
@flycash flycash merged commit 59a4fd1 into ecodeclub:dev May 21, 2023
@longyue0521 longyue0521 deleted the refactor/taskpool branch October 13, 2023 10:05
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.

2 participants