search

hashicorp / go-plugin

Golang plugin system over RPC.
Mozilla Public License 2.0
5.25k stars 450 forks source link

Make sure we wait for stdout/stderr pipes to be closed before calling Wait() #299

Closed gabivlj closed 8 months ago

gabivlj commented 8 months ago

Fixes https://github.com/hashicorp/go-plugin/issues/298

gabivlj commented 8 months ago

Hi @tomhjp! Could you take a look at these changes whenever you have a chance? Thank you!!

tomhjp commented 8 months ago

Thanks for the PR and the ping! I'd like to get a test for this fix, please could you share details on a repro? I can reliably repro if similarly to the previous issue I add a time.Sleep(time.Second) inside the stdout scanner loop, but I want to be sure that that's a faithful representation of the real scenario, i.e. is this a race condition that only pops up once in a while, or does it happen every time for your plugin?

tomhjp commented 8 months ago

I pushed a test that fails before this fix and passes after in https://github.com/hashicorp/go-plugin/commit/36c021db037a8f9873cca5d914b134fee84440d4. LMK if you think that looks like a good test case.

gabivlj commented 8 months ago

Thank you @tomhjp for the test! I appreciate it a lot. I think it's a good test case, as it will practically trigger that stderr exits before stdout and call the 'Wait()' syscall before stdout goroutine finishes.

We were encountering these logs "sometimes" in our error reports so it was just noise.