Frank,
Looks like I was not so clear on my presentation of the close(). I'll
have another go at it. I think some of the problem is the concept of
just what a close() call is. In this particular case it's just an
providing a FileObject an opportunity to do some additional processing,
it does not mean that the FTP server is finished with the FileObject.
Its easier to visual sometimes, so here is the ftp server in a
basic-sense. I only put in some of the major components, notice where
the FileObject is, and notice where the commands like STOR are:
+-------------------------------------------------------------+
| FTP Server |
+-------------------------------------------------------------+
+-------------------------------------------------------------+
| Server Context |
+-------------------------------------------------------------+
+----------+ +----------+ +------------+ +---------+ +--------+
| Command | | Message | | FileSystem | | User | | Config |
| Factory | | Resource | | Manager | | Manager | | |
+----------+ +----------+ +------------+ +---------+ +--------+
| |
+----------+ +------------+
| Commands | | FileSystem |
+----------+ | View |
| STOR | +------------+
+----------+ |
| ... | +------------+
+----------+ | FileObject |
+------------+
In the over-all scheme of the ftp server, the commands can be seen as an
intermediate level of a large operation. In most cases the commands are
some-what detached from the rest of the server operations. This is
similar to a large car factory with an assembly-line. The workers there
do not know if any particular car is special and should be handled
differently. The workers follow a pattern, repeating it for each car,
perhaps it putting in a part, or a bolt. They are not aware of how the
car got to them, nor what happens to the car after it leaves them. They
just do their repetitive task, same as the ftp-server commands.
In the FTP Server, the FileObject is a bit smarter than the "cars" and
likely the workers too :-). It knows how to handle most tasks that
involve it, such as creating input and output streams. It also knows
whether it's a directory or a file, and if a directory, it can list's it
children. It knows it's size or can obtain it, and so forth.
In the current code-set, commands that use a FileObjects stream's will
call a stream close() at the end of the command. The command STOR is an
example of one of these commands. At the end of STOR, the close() is in
a final block, to insure that the close() will always happen.
public class STOR {
...
} finally {
IoUtils.close(outStream);
}
}
The IoUtils.close() traps and ignores any exception that occurred with
the close().
In most cases the above scenario is okay, however in some cases,
ignoring exceptions on a close() is not okay. How to handle these?
One proposal is to add a FileObject close() method. Why? Because the
command -level operations like STOR have no idea about what to do for
any particular implementation of a FileObject. Who knows best about the
FileObject? the FileObject itself. So by adding a close() to the
FileObject, allows the FileObject the opportunity to do some additional
processing. One such opportunity is to close it's streams and handle any
exceptions.
Understand that a FileObject does not have to use the close() method, it
can just be empty. In any case the command-level operation, such as STOR
will call it anyway. Just to be sure that the stream really is closed,
the command-level operation will always call close() on the stream. If
the FileObject closed the stream, it's okay, the stream close() ignores
all exceptions.
The close() proposal would add the addition close() in the final block
of commands that use a FileObject stream. In addition, the order would
be to call the FileObject close() first and then the Stream close. This
will give the FileObject an opportunity to "personally" deal with any
tasks that it needs, such as closing a stream and catching the exceptions.
public class STOR {
...
} finally {
// let the FileObject do some post-process tasks
IoUtils.close(file);
// as-a-safety-check, try to close the stream
IoUtils.close(outStream);
}
}
Hopefully all of this makes sense? Again, keep in mind that the close()
is always done at the end of a command. In the current code only a
stream close is done, but it's always done at the end of the command. At
that point the command [STOR as an example] is done with the FileObject.
Also keep in mind that the close() in a FileObject does not have to do
anything. More importantly, calling the close() does not mean the FTP
Server is finished with the FileObject. The close() is just giving the
FileObject an opportunity to do some house-cleaning if it wants to.
Andy Thomson
Frank van der Kleij wrote:
Hi Andy,
I don't think it's conceptually clean to close the file object before
the stream is closed, it's very counterintuitive. I think the stream
must be closed first and than the file object. The close on the stream means
the transfer is finished, the close on the file object means you're done with
it altogether (in theory releasing it to an object pool or something similar).
In the handling you propose the exceptions are ignored anyway (IOUtils.close(FileObject)) so it does not resolve the need to inform the FTP client that the file transfer was not successful.
I do think that a close on FileObject is necessary but I don't think it is a
solution to my problem...
I don't quite follow your explanation of the assembly-line since I'm new here. In my view the outputstream is written and closed in the context of the STOR operation using a particular FileObject. If the close on the stream fails, the exception should be caught and an error code should be returned to the client. Likewise, if the close on the FileObject fails an error should be returned to the client too.
If you don't want to be bothered by exceptions on close I think it should be
the responsibility of the FileObject to muffle them; e.g. by wrapping the
OutputStream and ignore the exceptions on close there. In the current situation
the STOR command is explicitly killing all options to signal something went
wrong.
Frank
Date: Wed, 7 May 2008 21:18:14 -0600
From: [EMAIL PROTECTED]
To: dev@mina.apache.org
Subject: Re: (FTPSERVER-119) STOR command should not eat exceptions when
closing stream
Frank,
What should probably happen is that the FileObject.close() should be
called first, this then gives you the opportunity to close the stream
and handle the errors. The outStream close() would be called anyway,
because not everyone may want to handle their own close() so it insures
the stream is closed.
In the STOR command
...
finally {
// provide an opportunity for the FileObject to handle close
IoUtils.close(file);
// close the stream, it may have been previously closed, make sure
IoUtils.close(outStream);
}
In the FileObject
public void close() {
// close the stream(s) or raf
// do some other clean-up
}
This implies some changes in the FileObject issuance of the
createOutputStream() and createInputStream() if you wish to handle the
close on the stream(s). Or at least a change for the write-stream in any
case.
Once an operation [close() for example] leaves the FileObject, ie, it's
at the Command-level, the command-level does not know one file object
from another, nor if one FileObject is suppose to be handled
differently, it's just doing an assembly-line operation.
Does the above make sense on why a FileObject.close() was proposed?
Trying to handle individual FileObject tasks at the command level just
causes the "assembly-line" to slow down, and possibly break. Far better
to call the FileObject and let it deal with it's needs.
From your comments [excellent feedback], what should be done at the
command-level is call the FileObject.close() first, then the stream close.
Andy Thomson
Frank van der Kleij wrote:
Thanks for commenting on the issue.
The close on the file object seems a good idea in general but for me it won't
do the trick. The Apache VFS API does provide for close operations but they
serve a slightly different purpose.
VFS uses a stream based API to write; three objects are involved, a FileObject,
a FileContent and an OutputStream ( to write you'd have to do
fileObject.getFileContent().getOutputStream() ). Close on FileObject means
you're done treating the file, which is different from meaning that you're done
writing to it. The same goes for close on FileContent, which is supposed to
release all resources and closes InputStreams as well. Only close on the
OutputStream only means that all data is transferred.
For information, internally in VFS operations, e.g. on a copy operation, the close is only called on the outputstream too.
My point is that a close on the FileObject is not what I'm looking for; it is rather the handling of exceptions on close of the stream that concern me.
I doubt it is really necessary to ignore exceptions on the close of the stream.
I can imagine it was done because the close is called in a finally block and
handling exceptions there is rather ugly.
By doing the close before the finally the exceptions can be handled normally.
It would do a double close on the same stream, but streams are supposed to
support that kind of thing.
Best regards,
Frank
_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE!
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/
_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE!
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/
