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

Panic on closed channel #18

Merged
merged 4 commits into from
Oct 2, 2023
Merged

Conversation

sfleiter
Copy link
Contributor

I had a look on #15 since I was "lucky" and can reproduce this more than 95% of runs.

I had to restructure the code quite a bit but avoided to change any behavior.
Please report if you see anything.
For me this runs now successfully every time.

Most important part is to not close any channel.
In go you only have to close channels if you want to use that as a mechanism to tell the receiver no more values are coming, see f.e. https://go.dev/tour/concurrency/4

For everything else please look through the individual commits, that will probably be easier to understand the refactoring.

Copy link
Owner

@tg44 tg44 left a comment

Choose a reason for hiding this comment

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

I think this is smart, only one question below;

pkg/walker/walker.go Outdated Show resolved Hide resolved
expectedResultCount was wrong as soon one run of walk results in several additionas to jobQueue.
Counting the running jobs is more robust and also easier to understand.
@tg44 tg44 merged commit 10630bd into tg44:main Oct 2, 2023
1 check passed
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