Community
Participate
Working Groups
Build Identifier: Build id: I20100217-1031 We've recently been encountering InterruptedExceptions when executing multiple transactions on the same editing domain from different threads. On further investigation, it seems that we are hitting a few corner cases within the org.eclipse.emf.transaction.util.Lock class. In the uiSafeAcquire(..) method, we keep trying to acquire a lock using the AcquireJob until the lock is acquired, the user cancels the operation, or when another thread has yielded and given us the lock. This works perfectly. However: 1. If the job is unable to acquire the lock (for example, because another thread called syncExec), uiSafeAcquire tries to abort the job. The abort flag is thus made to TRUE, but since we keep looping in order to acquire the lock, the same job gets used again with its aborted flag set to TRUE from last time. The job thus thinks it has been cancelled and throws an interrupted exception which cascades all the way up to the application level. We can perhaps circumvent this by resetting the aborted flag once we loop again. 2. If the thread that is running the AcquireJob gets interrupted while it is acquiring the lock, then the job will return a cancelled status. However, if the job figures out that the lock has already been obtained previously by the same thread, it will return a re-entered status. Back in the uiSafeAcquired method, it detects that the job has returned a re-entered status, and tries to acquire the lock again quickly. Before we acquire the lock, however, a check is made to see if the thread has been interrupted (again, for example, if another thread calls syncExec). If there has been an interruption, this exception cascades to the application level because we do not catch interrupted exceptions in the uiSafeAcquire method. Reproducible: Sometimes
Created attachment 162908 [details] Fix Patch Here's a patch that fixes the corner issues.
To add some additional notes from this IBM product team: Please note that this is not a regression in EMF Transaction - this defect has been around for long time. However, changes in Project Explorer framework in Helios coupled with changes to our content providers have changed the timing and exposed the defect. Unfortunately, there does not seem a way to fix this outside of EMF Transaction. Even completely reverting all changes in PE and content providers (which I do not think is reasonable) would not really guarantee that the issue would be fixed.
Committed to HEAD for Transaction 1.4 (Helios).
*** Bug 228962 has been marked as a duplicate of this bug. ***
(In reply to comment #3) > Committed to HEAD for Transaction 1.4 (Helios). Forgot to make the status resolved.
You might want to change your pattern here. If you wanted to ignore the interrupted exception, you should at least propagate it. (Maybe a client that called you was depending on thread interruption, but since it called you, you could have swallowed the exception - and the client calling you will never know) The proper way to propagate an ignored exception is using the following pattern: boolean interrupted=false; try { while(condition) { try { wait(XXX); } catch(InterruptedException e) { interrupted=true; } } } finally { if (interrupted) Thread.currentThread().interrupt(); } === A note on your patch. === calling Thread.interrupted() in response to an InterruptedException is meaningless. Code that causes interruption usually does something like this. if (Thread.interrupted()) throw InterruptedException() That means, if a thread is interrupted, clear the interrupted state and throw an exception. That means calling Thread.interrupted() (to clear the interrupted flag) in response to an InterruptedException is meaningless.
Firstly, sorry for the late reply. I fully agree with you that it is best practice to not swallow interrupted exceptions (as per many papers and books). Swallowing interrupts like that may reduce the level of concurrency of the entire application. Especially if the locking phase in the Lock class is being done without a Job (as in with the patch I submitted as well as existing places) and is running on the Display thread, in which case our UI may stall during the locking phase. However, I have simply followed a similar pattern as the rest of the Lock class. There are many cases in the Lock class that swallow interrupted exceptions on a larger scale. For example, have a look at uninterruptibleAcquire(..) or uninterruptibleWait(..) methods. There may be a specific reason for doing so (e.g. EMF transactions should not be interrupted whatsoever?). I am quite in favour of not swallowing interrupts like that, but I do think that in order to allow for more concurrency during the Locking mechanism, we need to look at the Lock class as well as the entire EMFT transaction mechanism on a whole (and as a separate enhancement).
As a matter of fact, I see that you have already proposed this in early April :). Bug 308252
(In reply to comment #8) > As a matter of fact, I see that you have already proposed this in early April > :). > > Bug 308252 OK, I will comment further on this bug.
