cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

Takashi Yano takashi.yano@nifty.ne.jp
Tue Sep 14 08:07:16 GMT 2021


On Mon, 13 Sep 2021 22:15:25 +0200
Corinna Vinschen wrote:
> On Sep 14 04:37, Takashi Yano wrote:
> > On Mon, 13 Sep 2021 20:32:33 +0200
> > Corinna Vinschen wrote:
> > > I don't understand why calling fork_fixup on query_hdl should depend
> > > on the handle count.  If you duplicate a writer, you always have to
> > > duplicate query_hdl as well to keep the count, no?  Inheritence is
> > > handled by the O_CLOEXEC flag and fork_fixup will do the right thing.
> > 
> > I thought so, however, counting query_hdl cannot work as expected
> > without this check. The number of query_hdl opend seems to exceed
> > the number of writer.
> 
> If the write handle as well as the query handle are both opened,
> duplicated and closed in the same manner, they should never have a
> different count, unless the write side is inherited by a non-Cygwin
> client.
> 
> > There seems to be the case that handle is already inherited here
> > without fork_fixup. Any idea?
> 
> That should depend on the O_CLOEXEC setting, but identically for
> all handles in the fhandler.

I found the cause. set_close_on_exec() in fhandler_pipe is missing.
set_no_inheritance() calls for all adjunct handles are necessary.

> I pushed two more patches to topic/pipe in terms of inheritence,
> maybe that gives a clue?

I attached two additional patch for this issue.

> > > > +      if (!DuplicateHandle (GetCurrentProcess (), r,
> > > > +			    GetCurrentProcess (), &fhs[1]->query_hdl,
> > > > +			    GENERIC_READ, !(mode & O_CLOEXEC), 0))
> > > 
> > > This is a bug I introduced accidentally during testing.  This
> > > GENERIC_READ is actually supposed to be a FILE_READ_ATTRIBUTES.
> > > Sorry about that.
> > 
> > The PoC code uses PeekNamedPipe for query_hdl, so GENERIC_READ is
> > necessary I think.
> 
> Oh, right, that's how it's documented.  Funny enough, the descriptions
> of FSCTL_PIPE_PEEK does not mention any permissions at all.  I tried the
> permissions and it's FILE_READ_DATA which is required.

Thanks. I revised the patch and attach it the subsequent mail.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Cygwin-fhandler_base-dup-Reflect-O_CLOEXEC-to-inheri.patch
Type: application/octet-stream
Size: 1257 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-developers/attachments/20210914/2e0e3c61/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Cygwin-pipe-fifo-Call-set_no_inheritance-for-adjunct.patch
Type: application/octet-stream
Size: 1962 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-developers/attachments/20210914/2e0e3c61/attachment-0003.obj>


More information about the Cygwin-developers mailing list