[Evolution-hackers] Unexepected blocking reads in Camel

[Philip Van Hoof <spam pvanhoof be>]

Hey Camelers,

I'm not sure but it looks like if any Camel API call that causes a read
or a write that isn't wrapped by a CamelOperation, does a blocking read
or write that don't respect the timeouts.

For example (I'll comment inline with '##' characters):

ssize_t
camel_read (int fd, char *buf, size_t n)
{
	ssize_t nread;
	int cancel_fd;
	
	if (camel_operation_cancel_check (NULL)) {
		errno = EINTR;
		return -1;
	}
#ifndef G_OS_WIN32
	cancel_fd = camel_operation_cancel_fd (NULL);
#else
	cancel_fd = -1;
#endif
	if (cancel_fd == -1) {

##
## So in case the cancel_fd is not present, then do a normal read
##

		do {
			nread = read (fd, buf, n);
		} while (nread == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK));
	} else {
#ifndef G_OS_WIN32
		int errnosav, flags, fdmax;
		fd_set rdset;
		
		flags = fcntl (fd, F_GETFL);
		fcntl (fd, F_SETFL, flags | O_NONBLOCK);
		
		do {

##
## Else add the cancel_fd to a select, and either wait for a timeout, a
## successful read or the cancel_fd being set (right?)
##

			struct timeval tv;
			int res;

			FD_ZERO (&rdset);
			FD_SET (fd, &rdset);
			FD_SET (cancel_fd, &rdset);
			fdmax = MAX (fd, cancel_fd) + 1;

###
### The thing is the IO_TIMEOUT. It's respected in case of cancel_fd being
### set. It's not looked at in case cancel_fd was -1. Which is significantly
### different, of course (and probably unexpected behaviour too).
###

			tv.tv_sec = IO_TIMEOUT;
			tv.tv_usec = 0;
			nread = -1;

			res = select(fdmax, &rdset, 0, 0, &tv);
			if (res == -1)
				;
			else if (res == 0)
				errno = ETIMEDOUT;
			else if (FD_ISSET (cancel_fd, &rdset)) {
				errno = EINTR;
				goto failed;
			} else {				
				do {
					nread = read (fd, buf, n);
				} while (nread == -1 && errno == EINTR);
			}
		} while (nread == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK));
	failed:
		errnosav = errno;
		fcntl (fd, F_SETFL, flags);
		errno = errnosav;
#endif
	}
	
	return nread;
}



I created a patch for camel-lite that does the (what I think is)
expected behaviour.

http://tinymail.org/trac/tinymail/changeset/2349?format=diff


-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog