Gops hangs if there are too many Go processes to examine about gops HOT 17 CLOSED

It seems to me the simplest fix would be to make found buffered, with size concurrencyProcesses.

from gops.

I see now. Using len(pss) would work.

from gops.

This is impacting me as well. Was able to get around it temporarily by upping the magic number to 100 without a problem, though that's obviously not the right fix.

from gops.

@siebenmann @dbraley could you please check if this PR fixes this issue.

from gops.

Purely on inspection I believe that this will fix the issue I'm experiencing, however I believe it will also reintroduce issue #118. Since limitCh is now sent to in a new goroutine that has no other purpose, there's nothing to stop a flood of goroutines calling isGo() and using too many file descriptors at once.

from gops.

@siebenmann thanks for the insight. I have modified the code to use wg instead of limitCh.

from gops.

The change introduced above (see #125) moves the whole for-loop into a go routine (also suggested in the issue description). Ignoring the whitespace changes (git diff -b) is far the best way to understand the diff.

from gops.

I personally prefer to avoid buffered channels, but yeah -- it's possible as well.
UPD: See the comment below, where I explain why this actually isn't gonna work with concurrencyProcesses buffer size.

from gops.

Actually, did not initially notice that concurrencyProcess buffer limit proposed by @josharian. Imho it isn't gonna help. Imagine that we have more than concurrencyProcess Go processes. After the first batch of concurrencyProcess workers we will fill the output buffer, and will get to exactly the same situation as we are in right now. The only correct way in this case is to create a buffer of future len(results), which is unknown, and the nearest known value would be len(pss).

from gops.

But by that point, receiving from found will have commenced. In any case, there's no need to speculate. That's what tests are for. :)

from gops.

But reading from found happens after the main for-loop. It will never get there, because it will get stuck in the same way it gets stuck now.

from gops.

@josharian I've refactored my cl and made it testable.
UPD: Removed "Please try with len(pss) -- it will hang." Of course it will work. Sorry seems I have problems finishing reading sentences today (-;

from gops.

@josharian Added a fix-123-buffered branch. Please take a look and feel free to try it out yourself.
UPD: And yes, surely it would work with len(pss) as we've agreed earlier.

from gops.

Oups, sorry, forgot to push the changes. Now it's there.

from gops.

Also added a version, which creates less go-routines, and doesn't use buffered channels. Please take a look https://github.com/yarcat/gops/blob/fix-123-no-buffers/goprocess/gp.go (the branch is still based on a PR #125)

from gops.

Since this is impacting many users, instead of leaving comments on #124, I sent out #126.

from gops.

Sorry for the delay! Can confirm this fixed the problem for me as well, thanks @rakyll, @yarcat, @mseshachalam,
& @josharian !

from gops.