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);
