indygreg planned changes to this revision.
indygreg added inline comments.


> +        # separation. This prevents a whole class of potential bugs around
> +        # shared state from interfering with server operation.
> +

I'm having second thoughts about this.

The reason is that from a testing perspective (which is the primary driver 
behind this work), `read()` isn't very reliable because of timing issues. 
Depending on operating system settings, system performance, etc, operations 
like `read(-1)` can return a variable number of bytes because they return only 
what's available on the wire.

`write()`, however, is more reliable. When you `write()` to something in 
Python, Python makes as many system calls as necessary to ensure all bytes are 
delivered. So a `write()` at the Python level is mostly deterministic.

I think the concerns around process separation here aren't that significant. So 
I think I'm going to rework this (yet again) to spawn the SSH server in process 
and to only monitor I/O operations that are deterministic. This may mean only 
monitoring `write()` calls on pipes and *possibly* monitoring `readline()` and 
`read(N)`. But if we monitor `write()` on both peers since they are both 
in-process, then `read()` monitoring is redundant. That could be useful to 
debug behavior. But for tests demonstrating the wire protocol exchange, it's 
less useful.

  rHG Mercurial


To: indygreg, #hg-reviewers
Cc: sid0, mercurial-devel
Mercurial-devel mailing list

Reply via email to