Page MenuHomePhabricator

Make PhutilChannel::waitForAny more reasonable about detecting unselectable channels
ClosedPublic

Authored by epriestley on Dec 14 2013, 1:24 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 21, 3:39 PM
Unknown Object (File)
Tue, Dec 17, 8:33 PM
Unknown Object (File)
Sun, Dec 15, 4:14 PM
Unknown Object (File)
Fri, Dec 13, 10:13 AM
Unknown Object (File)
Fri, Dec 6, 11:59 AM
Unknown Object (File)
Fri, Nov 29, 3:26 AM
Unknown Object (File)
Mon, Nov 25, 10:14 AM
Unknown Object (File)
Nov 19 2024, 5:15 PM
Subscribers

Details

Summary

Ref T4189. Currently, channel getWriteSockets() methods return only blocked write sockets, and waitForAny() degrades to a busy wait if any channels are missing read and write sockets, assuming the channel is not a socket-based channel.

This means that if you have an unblocked, writable, socket-based channel and a blocked, readable socket-based channel, we always degrade to busy wait immediately.

An example of this occurs during git push:

  • We have one R/W channel through SSH to the git push on the user's computer.
  • We have one R/W channel through ExecFuture to the git receive-pack on the remote host.
  • When the client finishes writing, we close the "R" half of the SSH channel. This leaves it writable, socket-based, and unblocked for most of the push.

Instead, make getWriteSockets() methods return all write sockets, even if they're not blocked, and then ignore them if isWriteBufferEmpty() says we don't need to wait for them.

This is generally cleaner, and fixes several issues:

  1. The ssh-exec process no longer sits in a busy wait between the SSH read channel closing and the operation finishing.
  2. The stdin/stdout/stderr log in the SSH stuff can be simplified (next diff).
Test Plan

Ran git push of the Wine repository, saw nothing we own spend an unreasonable amount of time doing CPU stuff. Added a bunch of logging to confirm things are working reasonably under the hood.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped