microsoft / microsoft/dev-tunnels-ssh
Using `wantReply` on an `ChannelSignalMessage` can lead to deadlock
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 161
- Forks
- 34
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 2
Description
In both the C# and TS implementations, the ChannelSignalMessage explicitly sets wantReply to false before sending:
However, this change is done while the message is being written to binary, after the SshChannel checks the value:
This means that if you set wantReply on the message, the SshChannel will wait for a response which will never come, because it wasn't actually asked for.
This is easy to reproduce:
const signalMessage = new ChannelSignalMessage();
signalMessage.signal = 'INT';
signalMessage.wantReply = true;
// get a channel...
const res = await channel.request(signalMessage); // Deadlock!
There's no comment on the line that overrides the wantReply value so I'm not sure the purpose of it - reading the OpenSSH source it seems like wantReply is valid for a signal message and would be handled.
If this is intentional, it would probably just be best to remove the wantReply property from the ChannelSignalMessage. Otherwise, it would be good to remove the override.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/ts/ssh/messages/connectionMessages.ts around ChannelSignalMessage and src/ts/ssh/sshChannel.ts around the request check, then locate the corresponding C# implementation. Reproduce the wantReply=true example and verify that the chosen behavior no longer leaves channel.request waiting for a response that was not requested.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, typescript
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100