[Subclipse-dev] Another bug in new locking code in 1.1.9

Hi Mark,

On Thursday I introduced some of my teammates to using Subversion with Subclipse. We use locking for UML model files together with RSA 7.

The models are locked to prevent concurrent modifications which is not a so good thing to have with model files.

Anyway, as it turned out locking did not work as it worked before.

The problem is as follows:

When a lock request is made on a file (read-only in the workspace) that is already locked by another user the file will be read write after the lock command has run. The message about the other user having the lock appears in the console but the read only flag is always removed.

It took me some time to find out how this all works because I wanted to be able to debug down to the svnkit. I know have everything setup for debugging and can explain the problem.

The bug is in the new code of LockResourcesCommand:

        try {
            monitor.beginTask(null, 100);
            OperationManager.getInstance().beginOperation(svnClient);

            svnClient.lock(resourceFiles,message,force);
            // Ensure resource is writable. SVN will only change read-only
            // flag if the svn:needs-lock property is set.
            for (int i = 0; i < resources.length; i++) {
                                makeWritable(resources[i]);
                        }
        } catch (SVNClientException e) {
            throw SVNException.wrapException(e);
        } finally {
            OperationManager.getInstance().endOperation();
            monitor.done();
        }

The comment explains the intention of the code. I guess that an exception being thrown in case the locking fails was expected by the author of the new code. Unfortunately, this is not the case if the lock operation completes but returns with the information that some other user has the lock. In this case a handler set on the SVNClientImpl (when using SVNKit) is called with the information about the other user having the lock. This information is printed to the console using the JhlNotificationHandler and NotificationListener.

The code inside the LockResourceCommand will continue to execute and thus all files will be made read write.

So the fix would probly be something like this:

- Register a new handler before svnClient.lock(resourceFiles,message,force) is called and only call the makeWritable in case the handler signals success.

Thinking about this some more I really don`t see why it is necessary to make the files read write that were read only before and do not have the needs-lock property.
I could imagine someone using the lock operation explicitly on a resource that does not have the needs-lock but this would not make much sense for me because the other users wonÄt be prevented from making changes becaus there copies are not read-only in their workspaces.

I think definitly needs to be fixed for 1.1.10 since it maes using locks unsafe. In case the lock dialog is triggered implicitly when a user just begins to modify it and then saves it the modifications will be persistet altough the console shows the error message about the resource being locked.

One thing that I would also like to mention is that I would expect an error dialog in case the resource is already locked by someone else. It is difficult to understand for new users that they need to look at the SVN console to see why something is not working.

What do you think?

Regards,

Thomas
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subclipse.tigris.org
For additional commands, e-mail: dev-help@subclipse.tigris.org
Received on Sat Dec 23 12:03:23 2006