Re: Patch to work around flaky pop3 server behaviour

[Brian Stafford <brian stafford uklinux net>]

On 2001.06.09 12:06:49 +0100 Carlos Morgado wrote:

I haven't looked at the patch in too much detail so I can't really
comment on that except to say that sleep() isn't reliable in threaded
code and that when reading a non blocking socket the best thing
is something like this :-

	while ((n = read (sd, buf, sizeof buf)) < 0) {
		if (errno == EINTR)
			continue;
		if (errno == EAGAIN) {
			/* Block in poll() waiting for data to read. */
		}
	}

I.e. try reading what is availaable before calling poll().  This saves
one system call per read when data is already available.  Code like this
should always be used in conjunction with non blocking sockets when
reading or writing with a timeout is desired.  Blocking IO + signals
is OK in the traditional single thread Un*x program but even then the
technique here is better.

I personally would much prefer to see code that looks like this rather
than that in either of the patches.  For a start, the safe_read_char()
method is very inefficent, since it causes an overhead of 3 system calls
per *character* read.

Incidentally there should be no problem reading more than one character
at a time even with blocking sockets.  read() preserves packet boundaries
when reading from a socket.  Since the last characters in the packet
during command response exchanges will be the \r\n, there is no need
to read input a character at a time scanning for the line termination.
This is a property of protocols such as POP, IMAP, SMTP etc.

In fact, unless the POP server supports PIPELINING and the client uses
it, there is no possibility of a blocking read() trying to read beyond
the end of the line.  Reading a socket for lock step protocol exchanges
is not the same problem as reading a tty which really must be done
character by character (unless using half-delay mode which sockets don't
have).

You've probably guessed that I've a bee in my bonnet when it comes to
pipelining protocols and you'd be right.  The next libESMTP release
finally solves the deadlock problem.  Since this involves non blocking
sockets poll() and fcntl(), I feel I'm on home ground here.

While on the topic of libESMTP, it might be worth considering the use
of the socket buffering in siobuf.c for reading and writing in Balsa's
POP client.  (Perhaps I should add a POP client to libESMTP :)

A few other points:

> i like select/poll with timeouts better than signals and much better than
> fnctl tricks

Firstly, fcntl() is not a trick.  This is the standard way of getting
a non blocking socket, or indeed any type of non blocking file
descriptor. Other ways are to use send()/recv() (only works for sockets)
or to specify the O_NONBLOCK or O_NDELAY flag to open() (only works for
file descriptors returned by open().  Whatever the type of file descriptor,
fcntl() is the consistent way to get non blocking operation.

Secondly, poll() should be used in preference to select() in Posix code.
Select(), IMO, is an appalling piece of design.

Finally, use of non blocking reads and especially writes is the only
way to avoid a potential deadlock when buffering or pipelining command
response protocols.  I don't think this applies here.  However when coding
to avoid this deadlock, fcntl() and non-blocking sockets are not an
alternative to poll() with timeouts.  The two techniques must be used
in tandem.

Footnote: on the topic of signals, they can't be avoided either,
SIGPIPE must be caught or ignored in all robust socket applications.
If the remote closes its socket and the local host tries to write,
the process will be delivered a SIGPIPE which is normally fatal.

Regards
Brian Stafford.

PS. next release of libESMTP imminent - watch this space.  I'll have more
time to sort its Balsa interface when its out.