This is the mail archive of the cygwin-patches mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Oct 10 18:36, Christian Franke wrote: > Corinna Vinschen wrote: > >I was just looking into applying your patch when I got thinking over the > >change in select.cc once more. You're setting the connect_state from > >connect_pending to connected there when there's something to read on the > >socket. > > > >This puzzles me. A completed connection attempt should set the > >write_selected flag (see function peek_socket). > > No, peek_socket() does not change write_selected. It sets write_read if > write_selected is set. Uh, sorry, I mistyped. You're right, of course. > > The AF_LOCAL handling > >in the > > > > if (me->write_selected && me->write_ready) > > > >case in set_bits should cover this. What situation is your special case > >covering which is not already covered by the write_selected case? > > If only read status is requested via select()/poll(), write_selected is > always false and the connect_pending=>connected transition is never done. > > After a nonblocking connect(), postfix calls poll() with pollfd.events = > POLLIN only. If poll() succeeds, it calls recv(). This fails with ENOTCONN > because the state is still connect_pending. Oh. So it doesn't check if the connect succeeded? Does it check the poll result for POLLERR or does it explicitely check for revents==POLLIN? Hmm. [...time passes...] It looks like you catched a long-standing bug here. This isn't even AF_LOCAL specific. The original comment in the write_selected branch is misleading: The AF_LOCAL specific part is just the call to af_local_connect, not setting the connect_state. There was a previous, longer comment at one point which I shortened for no good reason in 2005: /* eid credential transaction on successful non-blocking connect. Since the read bit indicates an error, don't start transaction if it's set. */ However, If I'm not completely mistaken, your patch would only work in the aforementioned scenario if setsockopt(SO_PEERCRED) has been called. Otherwise the handshake would be skipped on the connect side and thus the handshake would fail on the server side. There's also the problem that read_ready may indicate an error. And POLLERR is only set if the socket is polled for POLLOUT so a failing connect would go unnoticed. In short, the whole code is written under the assumption that any sane application calling nonblocking connect would always call select/poll to check if connect succeeded in the first place. Obviously, as postfix shows, this is a wrong assumption. I'm not yet sure how to fix this, but I'll look into this next week. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat
Attachment:
pgpidpqJRn7eg.pgp
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |