Deadlock when using invalid local prefix path #13
Comments
|
From a quick look, I believe the cause is because |
nick-jones
added a commit
to utilitywarehouse/impi
that referenced
this issue
May 27, 2020
Deadlock was possible before because workers were spun up prior to the results channel being read. Using a buffered channel mitigates this to some extent, but it's still possible to trigger on large codebases. Realistically this needs to be reading the results channel in advance of the workers being spun up. This can end up getting quite complicated, as this means spinning up and coordinating another goroutine. Wait groups are quite handy for this - in particular the error handling errgroup is ideal. I've adjusted the code to spin up the necessary goroutines in order. The workers operate on a separate errgroup - this might seem quirky, but IMO it's the easiest way deal with the need to close the results channel when the workers are all done (if they were attached to the main group this would be a logistical issue.. something would need to understand that all workers have stopped). NB I relocated the channels from struct level fields to variables that are passed around. Happy to change this back, I just feel it's a little more clear to work with, particularly given channels will end up being closed (which would previously mean calling `Impi.Verify()` twice would result in an error). Resolves pavius#13
nick-jones
added a commit
to utilitywarehouse/impi
that referenced
this issue
May 27, 2020
Deadlock was possible before because workers were spun up prior to the results channel being read. Using a buffered channel mitigates this to some extent, but it's still possible to trigger on large codebases. Realistically this needs to be reading the results channel in advance of the workers being spun up. This can end up getting quite complicated, as this means spinning up and coordinating another goroutine. Wait groups are quite handy for this - in particular the error handling errgroup is ideal. I've adjusted the code to spin up the necessary goroutines in order. The workers operate on a separate errgroup - this might seem quirky, but IMO it's the easiest way deal with the need to close the results channel when the workers are all done (if they were attached to the main group this would be a logistical issue.. something would need to understand that all workers have stopped). NB I relocated the channels from struct level fields to variables that are passed around. Happy to change this back, I just feel it's a little more clear to work with, particularly given channels will end up being closed (which would previously mean calling `Impi.Verify()` twice would result in an error). Resolves pavius#13
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I noticed whilst testing changes over the weekend that a deadlock can occur if a local prefix is supplied that doesn't match any import (unsure what the exact reason is at this stage). This occurs on
v0.0.1so it's not linked to any recent changes. I'm happy to take a look at some point, just raising for visibility at the moment.The text was updated successfully, but these errors were encountered: