From 597de25969ebdeaafb7390984b5ce2c8782fd557 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 24 Aug 2011 12:48:19 -0500 Subject: [PATCH] ihandle: Fix IH_REALLYCLOSE for positional I/O Currently, ih_fdclose (which is called by IH_REALLYCLOSE), goes through every FD_HANDLE_OPEN FdHandle_t and closes it. If it finds handles that are FD_HANDLE_INUSE, it skips those and sets a flag on the parent IHandle_t. For non-positional I/O, any future opens cannot use these _INUSE handles, since _INUSE handles cannot be reused, and the handle will be actually closed when it is FDH_CLOSE'd. For positional I/O, the situation is different. Multiple threads can use the same _INUSE FdHandle_t, and so there is nothing currently stopping a thread from IH_OPEN'ing an ihandle that has been IH_REALLYCLOSE'd, and getting back an FdHandle_t that existed before the IH_REALLYCLOSE was issued. This is important, since IH_REALLYCLOSE is used on files that are deleted, and future IH_OPENs for the same inode must not use the cached file descriptor. Getting this wrong can cause data loss, since it can cause us to read from or write to a file descriptor referring to a deleted file, when we instead should open a new copy of that file. To fix this, we create a new FdHandle_t state called FD_HANDLE_CLOSING, which is set in IH_REALLYCLOSE if we encounter an FD_HANDLE_INUSE FdHandle_t. In IH_OPEN, we always skip FD_HANDLE_CLOSING handles, so we can never get back a cached file descriptor from before an IH_REALLYCLOSE call. Change-Id: I3188a18f7833950cf5454b3ffe4a4ce0c69e234f Reviewed-on: http://gerrit.openafs.org/5308 Tested-by: BuildBot Reviewed-by: Derrick Brashear --- src/vol/ihandle.c | 18 ++++++++++++++---- src/vol/ihandle.h | 3 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/vol/ihandle.c b/src/vol/ihandle.c index 7d8a17e..8c0e83b 100644 --- a/src/vol/ihandle.c +++ b/src/vol/ihandle.c @@ -325,6 +325,11 @@ ih_open(IHandle_t * ihP) /* Do we already have an open file handle for this Inode? */ for (fdP = ihP->ih_fdtail; fdP != NULL; fdP = fdP->fd_ihprev) { + if (fdP->fd_status == FD_HANDLE_CLOSING) { + /* The handle was open when an IH_REALLYCLOSE was issued, so we + * cannot reuse it; it will be closed soon. */ + continue; + } #ifndef HAVE_PIO /* * If we don't have positional i/o, don't try to share fds, since @@ -429,7 +434,8 @@ fd_close(FdHandle_t * fdP) IH_LOCK; osi_Assert(ih_Inited); osi_Assert(fdInUseCount > 0); - osi_Assert(fdP->fd_status == FD_HANDLE_INUSE); + osi_Assert(fdP->fd_status == FD_HANDLE_INUSE || + fdP->fd_status == FD_HANDLE_CLOSING); ihP = fdP->fd_ih; @@ -438,7 +444,8 @@ fd_close(FdHandle_t * fdP) * failed (this is determined by checking the ihandle for the flag * IH_REALLY_CLOSED) or we have too many open files. */ - if (ihP->ih_flags & IH_REALLY_CLOSED || fdInUseCount > fdCacheSize) { + if (fdP->fd_status == FD_HANDLE_CLOSING || + ihP->ih_flags & IH_REALLY_CLOSED || fdInUseCount > fdCacheSize) { IH_UNLOCK; return fd_reallyclose(fdP); } @@ -479,7 +486,8 @@ fd_reallyclose(FdHandle_t * fdP) IH_LOCK; osi_Assert(ih_Inited); osi_Assert(fdInUseCount > 0); - osi_Assert(fdP->fd_status == FD_HANDLE_INUSE); + osi_Assert(fdP->fd_status == FD_HANDLE_INUSE || + fdP->fd_status == FD_HANDLE_CLOSING); ihP = fdP->fd_ih; closeFd = fdP->fd_fd; @@ -783,7 +791,8 @@ ih_fdclose(IHandle_t * ihP) next = fdP->fd_ihnext; osi_Assert(fdP->fd_ih == ihP); osi_Assert(fdP->fd_status == FD_HANDLE_OPEN - || fdP->fd_status == FD_HANDLE_INUSE); + || fdP->fd_status == FD_HANDLE_INUSE + || fdP->fd_status == FD_HANDLE_CLOSING); if (fdP->fd_status == FD_HANDLE_OPEN) { DLL_DELETE(fdP, ihP->ih_fdhead, ihP->ih_fdtail, fd_ihnext, fd_ihprev); @@ -791,6 +800,7 @@ ih_fdclose(IHandle_t * ihP) DLL_INSERT_TAIL(fdP, head, tail, fd_next, fd_prev); } else { closedAll = 0; + fdP->fd_status = FD_HANDLE_CLOSING; ihP->ih_flags |= IH_REALLY_CLOSED; } } diff --git a/src/vol/ihandle.h b/src/vol/ihandle.h index 08e2d8b..8652f88 100644 --- a/src/vol/ihandle.h +++ b/src/vol/ihandle.h @@ -171,6 +171,9 @@ typedef struct FdHandle_s { #define FD_HANDLE_AVAIL 1 /* handle is not open and available */ #define FD_HANDLE_OPEN 2 /* handle is open and not in use */ #define FD_HANDLE_INUSE 3 /* handle is open and in use */ +#define FD_HANDLE_CLOSING 4 /* handle is open, in use, and has been + * IH_REALLYCLOSE'd. It should not be + * used for subsequent opens. */ /* buffered file descriptor handle */ #define STREAM_HANDLE_BUFSIZE 2048 /* buffer size for STR_READ/STR_WRITE */ -- 1.9.4