[PATCH 2 of 3 side-word (+4)] sshpeer: use a 'bufferedinputpipe' for standard output of the ssh process

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Jun 1 11:12:31 CDT 2015



On 06/01/2015 08:16 AM, Yuya Nishihara wrote:
> On Mon, 1 Jun 2015 08:01:55 +0900, Yuya Nishihara wrote:
>> On Sun, 31 May 2015 12:51:32 -0700, Pierre-Yves David wrote:
>>> On 05/31/2015 07:10 AM, Yuya Nishihara wrote:
>>>> On Sun, 31 May 2015 00:22:33 -0700, Pierre-Yves David wrote:
>>> The issue with the buffer+select is that we want to be able to perform
>>> such operation:
>>>
>>>    0) ssh: write "datadata"
>>>    1) hg:  select() -> hey! data to read
>>>    2) hg:  read(4) -> "data"
>>>    3) hg:  select() -> hey! data to read
>>>    4) hg:  read(4) -> "data"
>>>
>>> If you add the buffering, the 2) will actually read all available data
>>> in the buffer the second half. (3) will not see any data to read and
>>> block for ever (4) will never run (5) the internet kitten will never
>>> have his bowl filled.
>>>
>>> You can see the select usage here:
>>> http://42.netv6.net/marmoute-wip/mercurial/rev/b9213af62475
>
> Just an idea: because doublepipe requires main channel is a bufferedinputpipe,
> we won't need two file-like classes. They could be simpler if
>
>   - bufferedinputpipe is a fifo of string, like deque, not file-like
>   - and doublepipe is responsible for all read operations.
>
> For example,
>
>      class doublepipe(object):
>          def _wait(self):
>              if self._mainfifo:
>                  return (True, True)
>              select()
>              ...
>
>          def _call(self, methname, ...):
>              ...
>              while True:
>                  mainready, sideready = self._wait()
>                  ...
>                  if len(self._mainfifo) >= size:  # methname == 'read'
>                      return self._mainfifo.pop(size)
>                  if mainready:
>                      s = os.read(self._main.fileno(), chunksize)
>                      if not s:
>                          return self._maininfo.pop()  # EOF
>                      self._mainfifo.push(s)
>
> This will slightly complicate doublepipe, so I'm not sure which is better.

The original version was doing all in one. However, the buffer version 
using the 'os' module is isolated into the 'util' module (we avoid any 
'os' usage in any top module) and the douple pipe which enforce the ssh 
specific logic of "forward stderr as 'remote:' output" live in the ssh 
specific 'sshpeer' module.

Do you want me to update the commit message?

(note: at some point we will replace all this with thread).

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list