linux returns EAGAIN for closed ocserv interfaces

[Nikos Mavrogiannopoulos]

On Fri, 2014-09-26 at 13:33 +0100, David Woodhouse wrote:

> > > The close() system call should *never* fail to close the file
> > > descriptor. And as Linus points out, your force_close() hack is very
> > > broken in a threaded environment.
> > 
> > That doesn't matter much for ocserv as there are no multiple threads. It
> > was added as it looked reasonable for other OSes which may not behave as
> > Linux.
> 
> I'm not convinced there are any. I know of no other userspace which will
> retry a close() call. And it's hard to imagine how it could ever be
> necessary. The *flush* might fail, but closing the file descriptor is
> always easy enough because it just involves throwing away local state.
> 
> Also, we close all file descriptors when a process exits. If close() can
> fail, that makes a horrid mess of the task exit code path. I dread to
> think how this could ever be handled. I just don't think it can happen.

Niels, could you try this patch then (against master). If close() indeed
fails you should see "close failed:" in syslog. Otherwise we should
figure the real reason behind that issue.

btw. could that be reproduced the way David suggested?

regards,
Nikos

-------------- next part --------------
diff --git a/src/common.c b/src/common.c
index e88a560..faf65ff 100644
--- a/src/common.c
+++ b/src/common.c
@@ -75,6 +75,19 @@ static char tmp[32];
 	}
 }
 
+void force_close(int fd)
+{
+	int ret;
+	do {
+		ret = close(fd);
+		if (ret == -1) {
+			int e = errno;
+			syslog(LOG_ERR, "close failed: %s", strerror(e));
+			errno = e;
+		}
+	} while(ret == -1 && errno != EBADF);
+}
+
 ssize_t force_write(int sockfd, const void *buf, size_t len)
 {
 int left = len;
diff --git a/src/common.h b/src/common.h
index e17e908..57965fe 100644
--- a/src/common.h
+++ b/src/common.h
@@ -33,14 +33,7 @@ void *_talloc_size2(void *ctx, size_t size);
 #define PROTOBUF_ALLOCATOR(name, pool) \
 	ProtobufCAllocator name = {.alloc = _talloc_size2, .free = _talloc_free2, .allocator_data = pool}
 
-inline static
-void force_close(int fd)
-{
-	int ret;
-	do {
-		ret = close(fd);
-	} while(ret == -1 && errno != EBADF);
-}
+void force_close(int fd);
 
 ssize_t force_write(int sockfd, const void *buf, size_t len);
 ssize_t force_read(int sockfd, void *buf, size_t len);