Community
Participate
Working Groups
In RSE, create an FTP-Only connection. In Resource Perspective, create a new Project (type General, name foo). Below the new project, File > New > Folder > Advanced > Link to Folder Filesystem: RSE Browse to the FTP connection and any folder within it Below the EFS-backed folder, select a file and choose rename. Give a different name. A dialog "Renaming..." is shown and does not go away, even if cancel is pressed.
The following is going on here: 1. The RSEFileStore class does not implement the "move" method, so it falls back to the FileStore#move() default implementation. The default implementation performs "move" through calling "copy" then delete. 2. The "copy" operation opens a simultaneous InputStream and OutputStream. Moreover, EFS performs a getFile() operation, which results in FTPService#getFTPClient(), which sends a NOOP command; but the result of that NOOP command is never received, since the FTP client is currently busy performing the outputstream / inputstream. I think that we'll need the following to fix the situation: 1. FTPService#getFTPClient() needs a separate Thread with a Timer, which is capable of canceling the wait for the NOOP command when either the progress monitor asks for cancel or a given timeout is exceeded. If we do not do this in a separate thread, we'll always risk hanging all of Eclipse, since the getFTPClient() call does not react to the progress monitor cancel request. 2. FTPService#getInputStream() / getOutputStream() need to either a) do the Stream operations with a separate FTP connection each, such that getFile() etc. still works on the other FTP connection; or b) download the entire file to a local temp file and then return a stream on the local temp file. Option (b) seems to be the simpler one to implement although it's not nice that we'll end up having multiple temp files locally. 3. (Optional) As a performance improvement, RSEFileStore#move should be implemented to use the native IFileService#rename() or IFileService#move() operations for renaming and moving files on the remote side. This would avoid lots of data transfer. This is already requested bug #186312. It might be that measure (1) is not necessary any more when (2) is implemented properly; but I think I'd rather be on the safe side and have getFTPClient() at least react to progress monitor cancel requests.
Shouldn't RSEFileStore implement move instead of using FileStore#move() (solution 3) ? The package is internal, so it could be done for RSE 2.0.1. It looks like a cleaner solution as downloading and uploading a file with a different name to perform a rename doesn't look right. Also IFileService requires a move method, so it is already implemented in all classes implementing it and it could be called directly. If we keep the download/upload strategy for renaming, we could end up with bugs from users attempting to rename huge remote files, operation that is trivial in FTP (RNFR command, then RNTO command) and realizing that the huge file is downloaded and then uploaded again.
You are right, and if you read the previous comment again you'll see that in (3) I was already saying that bug #186312 is used to track exactly this. But I still think that the problem of multiple concurrent uploads/downloads must be fixed -- when the move issue is fixed, it will just be a little more difficult the provoke the hang but it will still happen if you e.g. dbl click on multiple large files through EFS to edit them in quick succession.
See also bug #198683 for a discussion how multiple parallel open streams should be handled by EFS providers
Then if bug 186312 is fixed this bug (renaming in FTP-based EFS folder) will be fixed as well. I agree that multiple uploads and downloads have to be supported and there's an enhancement request for it, bug 198636.
You will still hit the issue when you copy & paste a file on an FTP-backed EFS connection because it does the same (open download stream and open upload stream). The point is that it's very easy to get all of Eclipse hung up, and that's what needs to be fixed. I'm OK if copy&paste is not available, or download-multiple shows an error dialog but it must not hang Eclipse. So if this needs to be fixed anyways, I'm all in favor of fixing it properly.
Created attachment 75778 [details] patch creating the streams in different FTP sessions This patch allows renaming a file from a remote FTP server in EFS. It copies the file but I get the following error after the copy: org.eclipse.core.runtime.CoreException: This file system is read only: rse://LON-JAVIERMO01/folder1/folder2/renamedFile.zip. And no attempt to delete the original one. Martin: Can you reproduce it ?
(In reply to comment #7) Yes, I can reproduce the "file system read only" issue on every connection, even local. It's being tracked by bug #191589, I'm looking at it now.
Patch looks good over all, I just find it dangerous to catch away all exceptions in cloneFtpClient(). When anything goes wrong while creating the extra connection (and this is likely, since some FTP servers limit the available number of sessions so it might not let you in) you could end up with an UNCONFIGURED ftpClient. Or, an exception is thrown later during the transfer, but would make it hard to find the real cause. I think that cloneFTPClient() should just throw the exceptions it encounters. Another nice enhancement would be if the download() and upload() methods looked at how large the file to be transferred is (they do this already for progress reporting), and if it is larger then a given size, it used the openInputStream() / openOutputStream() methods to transfer the file in a separate thread, such that the original channel remains free while transferring the data in background. (likewise, the stream transfers could check size and download to a temp file if the file is small, just to avoid the extra streams if unnecessary; but that's for a future enhancement, or for a user preference per server, if it turns out that opening the extra channel is too much overhead).
Bug #191589 is now fixed and committed, please try again
The patch with the modifications from bug #191589 works fine and no error message is displayed. I'll improve the exception handling and commit the patch.
Improved patch applied. Added comment 4 in bug 180965 to use openInputStream() / openOutputStream() for file transfer operations.
Thanks for the fix! The exception handling was not perfect yet; I modified cloneFTPClient() to make sure that the clone gets properly disconnected in case anything goes wrong. Without that fix, it could happen that a half-connected ftp client would leak. Please review my fix in FTPService.java 1.36
Comment on attachment 75778 [details] patch creating the streams in different FTP sessions iplog- since Javier was committer since Aug 2006 already: http://dev.eclipse.org/mhonarc/lists/dsdp-tm-dev/msg00375.html
