Improve JSON validator co-process error handling #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I believe this was used to strip whitespace before the regex was pre-generated in hack/syntax_patterns.bash.
Closed
The json validator grep coproc interaction was working reliably when
grep was GNU grep, but it was not gracefully handling the termination of
the coproc in all cases. When a coproc exits immediately at startup,
accessing the coproc special variables (json_validator and
json_validator_PID) is subject to race condition, because bash unsets
the variables as soon as the coproc process exits. In the previous
implementation, we read the PID with ${:?} syntax, which could fail if
the PID var was unset by bash.
We now interact with the coproc in a rather more defensive way, handling
either or both of json_validator and json_validator_PID being unset at
creation; and then also handling SIGPIPE errors when later writing to
the FD of our writable end of the coproc's stdin. This is somewhat
fiddly, as it requires trapping SIGPIPE errors, otherwise bash exits
with 141.
We now have tests covering coproc failure at startup, after startup, and
IO read & write errors. We can also restart the coproc after an IO
error to recover. This could potentially occur when a read times out
under high system load.
Finally, several local vars in json.validate were not prefixed, so they
could collide with a caller's in= reference var. They're all prefixed
now.
We do this in json.validate now.
The JSON_BASH_GREP envar now defines the grep command used by json.bash when it starts a json validator co-process. The envar can be set to a : delimited list of paths/commands to use. It defaults to "ggrep:grep", so systems with ggrep installed with non-GNU grep will use ggrep.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR hardens the interaction with the JSON validator co-process, to avoid the error seen in #10 with
_PIDnot being set after starting a coproc.Related to this, we also now support using the
JSON_BASH_GREPenvar to change the grep command to a custom path, and we default toggrepif it's available.This resolves #12.