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

Avoid deadlock if Stop is called before Serve #2608

Merged

Conversation

chlunde
Copy link
Contributor

@chlunde chlunde commented Oct 30, 2020

Make a tiny state machine to detect the transition init -> stopped (not via serving)

Not 100% sure about this, I don't now this code and the origin issue mentions an issue thrift_processor too, but that might have been fixed already?

Fixes #2601
Related #103

Make a tiny state machine to detect the transition init -> stopped (not via serving)

Fixes jaegertracing#2601
Related jaegertracing#103

Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
@chlunde chlunde requested a review from a team as a code owner October 30, 2020 19:28
@mergify mergify bot requested a review from jpkrohling October 30, 2020 19:28
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

I restarted CI because it failed on another flaky test (certWatcher, #2586)

@codecov
Copy link

codecov bot commented Oct 30, 2020

Codecov Report

Merging #2608 into master will increase coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2608      +/-   ##
==========================================
+ Coverage   95.07%   95.10%   +0.02%     
==========================================
  Files         209      209              
  Lines        9366     9370       +4     
==========================================
+ Hits         8905     8911       +6     
+ Misses        385      384       -1     
+ Partials       76       75       -1     
Impacted Files Coverage Δ
cmd/agent/app/servers/tbuffered_server.go 95.00% <50.00%> (-5.00%) ⬇️
plugin/storage/badger/spanstore/reader.go 96.08% <0.00%> (+0.71%) ⬆️
cmd/query/app/server.go 90.16% <0.00%> (+1.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec03a2d...e4bc0af. Read the comment docs.

@yurishkuro yurishkuro merged commit a4076d5 into jaegertracing:master Oct 30, 2020
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.

cmd/agent/TestStartStopRace deadlocks
3 participants