Community
Participate
Working Groups
Build 20030115 IWorkspace.run(...) throws a CoreException saying that the resource tree is locked for modificatios, even if the IRunnable doesn't attempt to modify the resource tree. I would expect that it throws such a CoreException only if a resource modification is attempted.
Seems like either the calling code can change the tree or it can't. If it can then it should not be calling when the tree is locked (i.e., from a listener). If it can't then there is no point in using a workspace runnable. I don't understand the case where the proposed changes makes sense.
You are right Jeff, but until IWorkpace.isTreeLocked() was introduced 2 days ago, the client had no way to know if the tree was locked. Also this restriction is not specified on IWorkspace.run(...). So it appears that the implementation is not following the spec. Finally even if the client can now know if the tree is locked, it sounds like every client would have to put a guard before calling IWorkspace.run(...). It feels more natural if this guard is put in the implementation of IWorkspace.run (...)
I agree with Jerome, IWorkspaceRunnable should not be reserved to modifying resource operations, but rather as a generic facility. If the tree is locked, then do whatever is reasonable on your side, and if the client happen to modify a resource as some point, then he'll get the blame by then. This operation should only be a way to batch a set of operations together, not treated as a resource modifying entity per se.
Resolve one way or another by RC1.
One more question. Will the workspace runnable also eagerly take the workspace lock ? I would expect it not to.
Jerome/Philippe, can you please annotate the bug report with the details for the use case for this request? We're still having problems figuring out why code which doesn't modify the workspace is wrapped in a workspace runnable. Thanks.
Currently we have code to run JavaModel operations which looks like: if (operation.isReadOnly() || ResourcesPlugin.getWorkspace().isTreeLocked()) { operation.run(monitor); } else { // use IWorkspace.run(...) to ensure that a build will be done in autobuild mode ResourcesPlugin.getWorkspace().run(operation, monitor); we would like to get rid of the condition check, and ensure that in situations where we don't know upfront whether a resource change will occur (#isReadOnly would be false), we can rely on the workspace runnable to only lazily perform the necessary amount of work. In particular, maybe asking too much, I could imagine running a workspace runnable and it would only take the workspace lock once I attempt to modify a resource, as opposed to eagerly getting the lock though nothing in it requires it (since not changing any resource). It is only clients who decide about modifying resources or not inside the workspace runnable, not us. The current situation isn't so bad, since the obvious tree locked scenario can be foreseen, thus it could be downgraded to 2.2, since I cannot figure a major issue if clients are careful when invoking IWorkspace.run(...). Note that nowhere in IWorkspaceRunnable does it mention resource modification, thus I could simply use it to obtain progress reporting and/or cancellation behavior.
Just to confirm, there are 2 types of locks. There is the workspace lock which is controlled by the WorkManager. The workspace lock is aquired by IWorkspace.run() whether or not any resource tree modifications are taking place. This behaviour cannot change. The other type of lock is the information returned by the new API, IWorkspace.isTreeLocked(). This refers to whether or not the workspace tree is mutable. This bug report is a request that the workspace tree not be made immutable until absolutely necessary. So whether or not a workspace runnable makes changes to the tree, the workspace lock will always be aquired but the tree may or may not be locked. That being said, changes to this area of code are too risky for this late in the release cycle. (we equate changes to the workspace tree to changes in the classloader...not a good thing to do without a ton of testing) We would like to add this to the next release, though, pending further investigation into side- effects. Investigate for 2.2.
Understood, we should be fine. My comment on the workspace lock was just motivated by the fact that if you go lazy, then only the first actual resource change could take the lock anyway. This would result in finer grain locking, but it is likely a major candidate for causing deadlock as I saw much client code using workspace runnable to get the workspace lock, no matter if they modify resources in the end.
For concurrency work, we need to introduce a way to batch changes without acquiring a lock. This will have to be new API because some old clients will be using IWorkspace.run to prevent deadlocks (by always acquiring workspace lock before other locks). The other lock (Workspace.isTreeLocked), should go away with proposed changes to listeners.
John, can you confirm that we don't need to check if the tree is locked before using IWorkspace.run(...)? From what I'm observing using build I20031021, it looks like a CoreException is NOT thrown any longer if the tree is locked, but attempting to modify the tree will block until it is unlocked.
There shouldn't have been any change to this behaviour. If you try to run an IWorkspaceRunnable from a resource change listener you should still get an error. As always, if you run an IWorkspaceRunnable outside of a resource change listener, it will block until the necessary locks are available. My comment #10 about isTreeLocked going away is no longer valid, since the proposed changes for background resource change notification were rejected as too risky and too big of a breaking change.
After further investigation, it appears that the tree is marked as locked because another thread is in the middle of notifying resource changes after a build: 1. A resource is changed in thread 't1'. 2. Another thread 't2' is spawn to do the build. 3. At the end of the build, a POST_CHANGE notification occurs. 4. During the POST_CHANGE notification, the tree is locked. 5. In 't1' testing for isTreeLocked() returns true. However IWorkspace.run(...) can still be used. It doesn't throw a CoreException. 6. Tree modifications made in this IWorkspaceRunnable will just wait for the POST_CHANGE notification in 't2' to finish to proceed. So the question is how do we know that using IWorkspace.run(...) will throw a CoreException before calling it?
I have released a fix to HEAD. isTreeLocked now only returns true in the thread in which the resource change notification is occurring.
Moving back to inbox owner since I have no plans to work on this.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
