bug#49449: 28: TLS connection never gets to "open" stage

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sun, 11 Jul 2021 16:26:47 +0200
> Cc: larsi@gnus.org, 49449@debbugs.gnu.org
> 
> 
> [1:text/plain Hide]
> 
> 11 juli 2021 kl. 12.14 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > Did you succeed in understanding what else has to happen before that
> > flag could be safely reset?
> 
> Yes. The TCP connection needs to be established, the socket descriptor 
> removed from write monitoring (because it is now connected) and added to read 
> monitoring (so that we can get incoming traffic).
> 
> This suggests a second solution: remove the condition on line 3277 on the 
> grounds that since the TLS handshake succeeded, we are evidently connected; 
> then remove the write monitoring and add the read monitoring before calling 
> the sentinel:
> 
> 
> [2:application/octet-stream Show Save:alt.diff (649B)]
> 
> 
> [3:text/plain Hide]
> 
> 
> > And anyway, if those conditions are not yet set, I wonder why are we
> > calling finish_after_tls_connection at that place?
> 
> There's no harm in calling `gnutls_handshake`; it will just return E_AGAIN if 
> the connection hasn't been established. On the other hand there's little 
> point in doing so until we have a connection.
> 
> Which suggests a third solution: do the handshake right away after 
> establishing the connection. That would go into the code somewhere before 
> line 5900, which right now is a condition that I don't quite understand. I 
> think Lars wrote it but apparently forgot all about it (happens to everyone 
> once in a while).

Thanks for the explanations.

> I still favour the less intrusive patch posted previously (adding a condition 
> at line 5235) since it avoids duplication; there is already far too much of 
> that in the code (everything seems to be done in at least two places). The 
> code is obviously in the need of restructuring, but we shouldn't conflate 
> that effort with fixing this specific bug.

I tend to agree.