A PR submitted for tunny

background

Last week, I wrote an article Go, the ants daily library, and deeply analyzed the ants the goroutine pool 060cb3243904d3. Repeatedly read many times panjf2000 on ants article of origin - GMP-depth analysis of concurrent scheduler hand a high-performance line and the pool goroutine , I feel full harvest. This article is of great reference value for understanding Go's goroutine concurrency mechanism, and it is strongly recommended to read it. Then I spent a few hours reading the ants in detail, and the code was written very well and very beautifully. Then I wrote again essay analyzes the ants source, see ants source Appreciation . In the process of writing ants and in-depth reading of the source code, online materials mentioned the implementation of the goroutine pool in the Go language, and often brought the library tunny So, I went to study the tunny , and produced an article Go, a library of tunny every day.

When reading the tunny , I found that there is a problem with the implementation of the method. I also pointed out Go daily tunny

principle

We know that there is a pointer to an array in the slice structure. Assuming tunny at the beginning, the schematic diagram is as follows:

Each element in the array is a pointer to a worker structure. Then we used s := s[:4] shrink the volume, and it became as follows:

Now the last element cannot be accessed through the slice, but it is referenced by the underlying array again, and cannot be cleaned up by the gc of the Go runtime, and the 360 software housekeeper will not work. This is a memory leak. Although this leak is not serious, one is because the amount is unlikely to be large, because the number of workers is limited, and the other is that the original worker can be released when the location is overwritten after the next expansion (because there is no pointer to refer to it).

ants source code, but the reduced elements are set to nil . E.g. worker_stack.go in reset() Method:

func (wq *workerStack) reset() {
  for i := 0; i < wq.len(); i++ {
    wq.items[i].task <- nil
    wq.items[i] = nil
  }
  wq.items = wq.items[:0]
}

PR

Determined the problem. I fork the warehouse tunny Click the fork button:

Then download my own warehouse after fork:

$ git clone [e-mail]:darjun/tunny.git

Modify the code, commit, push to my remote warehouse. Here are the changes I made:

Then go to the tunny source warehouse and click the Pull Request button. Then click the New pull request button on the right:

Create a new PR, then click compare across forks , select my fork:

Select the submission that I made the modification, and then fill in the information and submit, because I have already submitted the PR. There is no difference in this comparison.

Then wait for the authors to merge. A day later, the author merged this modification:

to sum up

It is not difficult to submit a PR on GitHub. You can submit a PR from a new feature to a small spelling error. I believe many people have heard of it. Linus Torvalds personally merged a PR from an 11-year-old child about spelling errors in the Linux source code comments.

Many star projects on GitHub are not perfect, and there are definitely many areas that deserve improvement. Therefore, you need to be skeptical when reading the source code, and don't regard anything as a standard.

If you find a fun and useful Go language library, welcome to submit an issue on the Go Daily Library GitHub😄

reference

1. tunny GitHub：github.com/Jeffail/tunny
2. ants GitHub：github.com/panjf2000/ants
3. Go has a library GitHub every day: https://github.com/darjun/go-daily-lib

I

My blog: https://darjun.github.io

Welcome to follow my WeChat public account [GoUpUp], learn together and make progress together~