[PVFS-developers] IOD File Descriptor Leak

[Rob Ross]

Hi Don,

I see what you're talking about; the file structure is allocated in 
iod.c:do_open_req() but the sock field isn't set until jobs.c:do_rw_req().

This is a problem because the flist_purge_sock() call then cannot 
correctly remove the file structure on socket close.

However, your assertion that do_rw_job() just grabs the first finfo isn't 
correct.  That search is based on both the ino and the capability number, 
which is different for every client.

I'm not sure that there is still a clear reason to have multiple file 
descriptors for each socket; however, it is currently partially used for 
permission purposes, and it isn't clear to me how moving to a single FD 
would change how the kernel perceives what the iod is doing.

I'm checking in a single-line fix that associated the socket with the file 
structure at open time.  I think that this will fix the leak.  If you want 
to investigate changing to using a single FD for each file, that's cool, 
but I'm a little hesitant to switch that up without significant testing.

Thanks for the leak catch!

Regards,

Rob

On Tue, 16 Sep 2003, Porter Don wrote:

> I have been noticing on some of the iods what appears to be a file
> descriptor leak.  In looking at the iod code, it seems to be happening when
> a job cancels between opening a file and doing a rw job.  The socket number
> in the finfo struct is initialized to -1 and doesn't get set to a real
> socket until a rw job gets it from the list.  If this field never gets
> initialized, the finfo will not be cleaned up when the socket closes.  The
> leak seems to be introduced because right now the do_rw_job function just
> grabs the first finfo for the inode it wants and assigns its socket if it is
> uninitialized, completely ignoring all but the first finfo on the linked
> list.
> 
> This raises the question, is there any reason to have multiple file
> descriptors for the same inode file at all?  The iods are single threaded
> and are effectively sharing one anyway.  It seems that perhaps changing the
> finfo structure to keep a linked list of sockets as a sort of "reference
> count" (like the mgr) might be more efficient and plug up this leak.  I
> suppose this would also need to be coupled with having something in
> check_socks that would clean up unclaimed finfo structs after a period of
> idleness.
> 
> Anyway, I am about to start working on this, but I thought I would bounce it
> off of the list and see if anyone had any insights.