Tufin / oasdiff

OpenAPI Diff and Breaking Changes
https://www.oasdiff.com
Apache License 2.0
689 stars 59 forks source link

Race condition while parsing command line arguments if telemetry is enabled #503

Closed c-garcia closed 4 months ago

c-garcia commented 6 months ago

Describe the bug

In Fedora Linux 38 systems (both aarch and x86_64), when running multiple parallel instances of the command:

oasdiff breaking openapi.orig.json openapi.json

Some of then (2 or 3 out of 50, in our experiments) return error 102 to the operating system and show the text:

Error: failed to load base spec from "file": open file: no such file or directory

Please, note that the error is points to the file named file to be missing. It does not mention the real name of the file openapi.orig.json as is normally happens when the file is actually missing.

If telemetry is disabled, this behavior is not observed. That is, the command below works as expected.

OASDIFF_NO_TELEMETRY=1 oasdiff breaking openapi.orig.json openapi.json

To Reproduce

Expected behavior

We expected the command to not return that a file was missing, when in fact it was not.

Desktop (please complete the following information):

Additional context

We have observed that this behavior seems to be caused by calling SendCommand in the preRun function. SendCommand seems to be visiting the cmd.Flags within a goroutine. Unfortunately, pflags.Visit seems not to be thread-safe and actually updates the internals of the data structure. While this happens, cobra is potentially accessing it to perform some validations such as ValidateRequiredFlags (see the code here, please). This is a potential candidate to explain the behavior and why the error is difficult to reproduce.

reuvenharrison commented 6 months ago

Thanks for the detailed replication. I'll take a look at this.