Add comment explaining background thread usage for plugin calls (#7878)

~~I happened to be reviewing our uses of `thread::spawn()` and came to
the conclusion that we're spawning a thread unnecessarily for plugin
calls. We were basically doing this:~~

~~1. Spawn a background thread to send data to the plugin over stdin~~
~~2. Immediately do a blocking wait for the plugin's response~~

~~As far as I can tell, there's no point in spawning a thread for 1 (and
it may harm error handling) given that we're blocking right away for the
response.~~

**Update:** the logic is correct, as confirmed by @WindSoilder
[here](https://discord.com/channels/601130461678272522/855947301380947968/1072743414795350037).
I've added a comment explaining the thread usage.
This commit is contained in:
Reilly Wood 2023-02-08 08:53:44 -08:00 committed by GitHub
parent 2917c045fb
commit f9b5d8bc5e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -91,10 +91,10 @@ pub(crate) fn call_plugin(
) -> Result<PluginResponse, ShellError> {
if let Some(mut stdin_writer) = child.stdin.take() {
let encoding_clone = encoding.clone();
std::thread::spawn(move || {
// PluginCall information
encoding_clone.encode_call(&plugin_call, &mut stdin_writer)
});
// If the child process fills its stdout buffer, it may end up waiting until the parent
// reads the stdout, and not be able to read stdin in the meantime, causing a deadlock.
// Writing from another thread ensures that stdout is being read at the same time, avoiding the problem.
std::thread::spawn(move || encoding_clone.encode_call(&plugin_call, &mut stdin_writer));
}
// Deserialize response from plugin to extract the resulting value
@ -154,6 +154,9 @@ pub fn get_signature(
// Create message to plugin to indicate that signature is required and
// send call to plugin asking for signature
let encoding_clone = encoding.clone();
// If the child process fills its stdout buffer, it may end up waiting until the parent
// reads the stdout, and not be able to read stdin in the meantime, causing a deadlock.
// Writing from another thread ensures that stdout is being read at the same time, avoiding the problem.
std::thread::spawn(move || {
encoding_clone.encode_call(&PluginCall::Signature, &mut stdin_writer)
});