Re: Issues with closing destination file in SmbFile.copyTo0()

On Thu, Jun 19, 2014 at 9:56 AM, Vella, Shon <[email protected]> wrote:
> 1. There are no guarantees that the last block has been written by
> writer thread before Trans2SetFileInformation is sent or dest.close()
> is called.
> 2. If there are any exceptions thrown during the read/write loop or
> when sending Trans2SetFileInformation, dest.close() will not be called
> at all, leaving it open until the session is dropped.
>
> I've created a patch that addresses both problems. Note that the patch
> builds depends on a previous patch
> (http://article.gmane.org/gmane.network.samba.java/9348) that I've
> submitted.
<snip hard to decipher patch>

Hi Shon,

Nice catch.

Can you please just attach your entire SmbFile.java file (send it to
me directly)? It would make it easier for me to see the totality of
your changes (including those from October). Note that I try not to
blindly apply patches. I just look at what they do and then do what I
think is right.

Incidentally, another possible fix might be to have a
WriterThread.isDone() method that just checks if n == -1 and then use
that to break out of the copyTo0 for loop. I'm not saying that your
fix isn't better. It's just what popped into my head when I looked at
the code and without seeing your fix first.

As for ensuring files are closed, that can certainly be improved. But
bear in mind that if an error occurs writing the file, it may turn out
that trying to close the file will also fail such as because of a
network failure. So I don't think it is possible to completely
eliminate open files (although if you set ssnLimit = 1 that should
cause each conn to close after soTimeout and thus release any file
handles). But the fix is probably just to move the dest.close() into
the finally block (with it's own try catch to just ignore whatever the
outcome is). At least whatever needs to be done should be in the
finally.

Good work,

Mike

-- 
Michael B Allen
Java Active Directory Integration
http://www.ioplex.com/