[GitHub] [commons-vfs] boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel
boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel URL: https://github.com/apache/commons-vfs/pull/36#issuecomment-487551862 > The `session` ivar is not final and gets written in the private method `ensureSession()`, so it is possible for it to get initialized multiple times in a race-condition. The method `getChannel()` can end up writing to both `session` and `idleChanne`, so both values need to be published to other threads. The simplest way to do this is to synchronize the `getChannel()` method. > > Thoughts? Well, in that case there is still a race-condition because of the `executeCommand` method which also calls `ensureSession` so I guess it also should be `synchronized`...? > I do not see anything in that PR about concurrency. That PR just fiddles with POMs. What am I missing? This PR has nothing to do with concurrency but as I said @abashev's fork of VFS has some fixes in it which he wants to merge - and this PR is just the first one of a few to come as far as I understand. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-vfs] boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel
boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel URL: https://github.com/apache/commons-vfs/pull/36#issuecomment-487548091 @garydgregory, @ottobackwards - in that regards, please take a look at [this PR](https://github.com/apache/commons-vfs/pull/54) which is by @abashev. In his S3 provider he uses a fork of VFS in which he claims he's fixed a number of concurrency issues. He wants to merge them in the upstream repo (this one) so please take a look at the PR I linked (and accept it/comment on it) so he can continue on with the real issues he thinks he's resolved. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-vfs] boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel
boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel URL: https://github.com/apache/commons-vfs/pull/36#issuecomment-487544065 @ottobackwards - well, I've been using VFS for a while now in a multi-threaded environment and it mostly works fine. There are some issues here and there but I think they will be resolved one by one and all will be good in the end. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-vfs] boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel
boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel URL: https://github.com/apache/commons-vfs/pull/36#issuecomment-487393002 @garydgregory - I'm not familiar with the specific code here but the diff that I see here is totally safe - `idleChannel` is `volatile` and it's the only variable on which DCL is performed. This is safe since Java 1.5. Why do you think `session` also has to be? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-vfs] boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel
boris-petrov commented on issue #36: fix for VFS-662: SftpFileSystem has Thread-safe issue about idleChannel URL: https://github.com/apache/commons-vfs/pull/36#issuecomment-487311364 @garydgregory - DCL has not been broken for [a long time now](https://stackoverflow.com/a/3580658/4478636) as long as you use a `volatile` instance variable. :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
