Community
Participate
Working Groups
I have a plugin that calls FileServiceSubSystem.getRemoteFileObject on a linux system. When the system is not connected, this code throws an NPE: java.lang.NullPointerException at org.eclipse.rse.services.dstore.AbstractDStoreService.getMinerElement(AbstractDStoreService.java:75) at org.eclipse.rse.services.dstore.AbstractDStoreService.getMinerElement(AbstractDStoreService.java:68) at org.eclipse.rse.internal.services.dstore.files.DStoreFileService.getElementFor(DStoreFileService.java:1297) at org.eclipse.rse.internal.services.dstore.files.DStoreFileService.getFile(DStoreFileService.java:767) at org.eclipse.rse.internal.services.dstore.files.DStoreFileService.getUserHome(DStoreFileService.java:889) at org.eclipse.rse.subsystems.files.core.servicesubsystem.FileServiceSubSystem.getUserHome(FileServiceSubSystem.java:264) at org.eclipse.rse.subsystems.files.core.servicesubsystem.FileServiceSubSystem.getRemoteFileObject(FileServiceSubSystem.java:187) I expect RSE to try and connect (i.e. the password dialog should come up) and if I press Cancel then this call should return null (or something appropriate) instead of throwing an NPE.
Throwing an NPE is certainly the wrong thing, but I'd be very careful with an implicit connect during getRemoteFileObject(). The risk with implicit connect is that when a user manually disconnects a connection, some client code still wants a file object reference after disconnect so the system would immediately reconnect. Currently, API Docs of IRemoteFileSubSystem#getRemoteFileObject() don't tell whether it does an implicit connect or not. The docs just say that it can be a long-running operation, and that it may throw a SystemMessageException. Because of these API docs, and we don't want to change API if we don't have to, I think we should never return null, but throw a SystemMessageException if anything goes wrong. We might want to introduce a special subclass of SystemMessageException to handle the "not connected" case. API Docs should be extended to explain what happens if the subsystem is not connected. This would be a great candidate for a Unit Test, such that we can verify all our subsystems (ssh, ftp, dstore) show the same behavior on a disconnected system. Dave - what do you think?
(In reply to comment #1) > ... > The risk with implicit connect is that when a user manually disconnects a > connection, some client code still wants a file object reference after > disconnect so the system would immediately reconnect. > For the older IBM RSE, we did do the implicit connect whenever a subsystem query API was called and we were not connected. IBM products were able to make the assumption that such calls could be made without having to worry about the connected state. The subsystem method we used is still around - checkIsConnected(). As far as the client code referencing a file object after disconnect, we didn't have a problem - only when the user tried to do some kind of query with the referenced object did we implicitly reconnect. > I think we should never return null, but throw a SystemMessageException if > anything goes wrong. We might want to introduce a special subclass of > SystemMessageException to handle the "not connected" case. API Docs should be > extended to explain what happens if the subsystem is not connected. > If we do decide to put the implicit connect back, then we're unlikely to run into the case of having to return null or throw the system message exception. Personally, I'm in favour of bringing back implicit connect for getRemoteFileObject() calls (and possibly others) since it's intuitive, convenient and historical for IBM products.
Created attachment 81062 [details] patch for implicit connect and dealing with NPEs in dstore file service I've attached a patch that deals with NPEs in the dstore file service layer and also does implicit connect in FileServiceSubSystem.getRemoteFileObject() if required.
Ok, if it's always been like that (implicit connect) then we better fix it rather than changing it all. I'd love to see JUnit tests for the implicit connect, such that we can verify that all FileService implementations show the same behavior.
I've committed the code to cvs. I ended up changing checkIsConnected() to take a progress monitor since, by the way it is used, we should be running the connect in the same job as the caller operation. I've also added a test case, FileSubsystemConsistencyTestCase, for comparing behaviours like this using the different service implementations. Currently FTP is failing, so I'll need to figure that one out.
The problem I had with ftp was in the testcase. I'll close this now.
Marking as breaking API since we changed ISubSystem#checkIsConnected() into ISubSystem#checkIsConnected(IProgressMonitor)
I just ran into another variation of this problem. A call to SubSystem.resolveFilterStrings(*) does not prompt for a connection when the subsystem is not connected. I believe the old IBM server used to prompt.
Ankit please file a new bug for your issue.
