Re: Asynchronous DNS

[Eli Zaretskii]

> From: Alain Schneble <address@hidden>
> CC: Lars Ingebrigtsen <address@hidden>, <address@hidden>
> Date: Sun, 7 Feb 2016 01:34:14 +0100
> 
> >> I kinda think we've gotten as far as we're going to get in this
> >> discussion, and that it's time to merge.
> >
> > I'm sorry, but I disagree.  I think the APIs that expect a fully
> > functional process object should wait for the DNS resolution to
> > complete, before they do anything.
> 
> FWIW, the process-send-* functions do not wait for an async connection
> to complete either, even without async-DNS-feature turned on.  They
> throw an error if called before the async connection completed, IIUC.

That's not my reading of the code.  Perhaps I'm missing something.
Could you tell the details -- where do you see this?

What I see is this: process-send-string calls send_process, which
signals an error if either the status of the process object is other
than 'run' or its output file descriptor is invalid.  But
make-network-process takes care to arrange for both of these: it calls
make_process, which sets the process status to 'run' and sets up its
output file descriptor to a valid value (the socket we got from the
call to 'socket').  It does that even if the connection is
asynchronous and has not yet completed.  So I don't expect an error in
this scenario with the existing code.

If the connection is asynchronous, and is not yet completed, then
send_process will wait in a loop, because the write to the socket will
return EWOULDBLOCK or EAGAIN.

Did I miss something?

> And the documentation does not mention anything about this behavior.

Well, if I'm right, it shouldn't ;-)

> It seems to me like it is a matter of definition to say which functions
> should work properly also before an async connection is established and
> which not.

I definitely hope that is not true.  We couldn't have reliable network
connections if it were true.

> If backward compatibility were not an issue, we could also adopt a more
> restrictive approach where errors would be thrown in many of the
> functions accepting a (async socket) Lisp process and requiring a valid
> infd/outfd before async connection completion.

I think this would make network programming in Emacs a nuisance.

> Documentation of `make-network-process' could say something like "if
> :nowait is t, process returned shall not be used in any call before
> async connection has completed.  Consider performing any required
> process configuration withing the SENTINEL".  Oops, just realized
> that this is what Lars more or less put in the doc string for the
> 'dns case...

Please consider the plight of a programmer who needs to implement such
a connection.  It would be much more complex and hard to get right
than what we have today.  E.g., look at smtpmail.el: it opens the
network stream, sets up the process filter and encoding, and
immediately proceeds to sending the HELO command to the server.
Imagine how much more complex this simple job would be if it needed to
jump through hoops as you described above.

> Just as an overview, here is an extract of functions relying on a valid
> infd/outfd and hence would be candidates that could either throw errors
> or, as you suggested, synchronously wait for the DNS resolution to
> complete.  There are quite a few though:

They all should call a single function that waits calling gai_error
until the connection completes, and signals an error if it completes
with a failure indication.  That's a very simple and straightforward
addition to the preamble of all those functions, and it should fix all
those problems in a way that is 100% backward compatible.

> >> There should be no impact on any existing use.  It should be 100%
> >> backward compatible.
> >
> > But it isn't, as was demonstrated by ERC.  And it cannot be 100%
> > backward compatible, unless the above-mentioned waiting is added to
> > the affected APIs.
> 
> I think what Lars meant was that with :nowait to t, client code should
> be 100% backward compatible, as async connection establishment is still
> handled the same way internally.

Yes, I understand.  But I don't think this is a proper way to be
backward compatible.  With my suggestion, code remains backward
compatible even if it uses :nowait.  This is better, IMO, because it
leaves more applications backward compatible.  For example, imagine
that :nowait comes from some higher level, so the level that arranges
the connection has no idea whether it needs to do the simple thing or
the complex one.  What Lars suggested means that every application
that doesn't pass literal parameters to make-network-process will now
have to analyze the parameters passed by the caller, to see if :nowait
is there and what is its value.  I don't expect package maintainers to
love this.

> Only when :nowait is 'dns, the new async DNS resolution is activated.
> And only in this case, we might run into /new/ issues with calls to
> functions accepting a Lisp process, such as the issue revealed by ERC
> before Lars introduced 'dns.

I think the solution I suggest will provide backward compatibility
even if :nowait is 'dns.  So the new issues are avoided.

> On one hand, I see the practical approach with the 'dns async mode to
> circumvent these compatibility issues.  On the other hand, I don't think
> it's a good choice from a interface/usage point of view.  I think it is
> not clear for a user why she has to care about whether both async DNS
> and async socket connection or only the latter shall be used and why the
> process behavior is different.  This distinction only makes sense with
> the knowledge of how it's implemented internally, I think.

Yes, and so my suggestion is better, I think, as it avoids all of
that.  An application that didn't make the necessary changes will not
be able to take the full advantage of the async DNS, but it will still
work.

> I guess changing the semantics of :nowait t, to say that clients shall
> not use the returned process before connection is established, is out of
> discussion, right?  Or do you think that this would even be a ridiculous
> restriction (even without the backward compatibility issue)?

Yes, that'd be a terrible nuisance, IMO.  Users and Lisp programmers
will hate us if we do that.