[news.eclipse.tools.emf] Re: EMF Transaction: Notification list will not

["Christian W. Damus" <cdamus@xxxxxxxxxxxxx>]

Hi, Ben,

If the other thread is using a transaction, then I think there wouldn't 
be any concurrency issue in the notification list.  As such, this isn't 
any different from unsafe concurrent access in other APIs.

That said, this is a layer on a framework (EMF) that integrates all 
kinds of third parties that are not transaction-aware (and arguably 
cannot be), so perhaps it behooves the transaction API to ensure safety.

What is your TransactionUtils.writing(...) method?  Is that like 
TransactionalEditingDomain.runExclusive(...) but not read-only?

The main concern that I have with addressing this is that it will 
introduce more synchronization, so performance may take a hit.  I don't 
know how much to expect, though.

Cheers,

Christian


Ben wrote:
> Hi,
>
> We have seen that when a thread has a transaction open, if a separate 
> thread causes a notification to be sent through the EMF model, then the 
> transaction's notification list will not be safely modified.
>
> Here are a couple of stack traces that we have captured when the problem 
> occurs:
>
>
> Uncaught exception during pre-commit trigger processing
> java.lang.NullPointerException
>   at 
> org.eclipse.emf.transaction.NotificationFilter$2.matches(NotificationFilter.java:65) 
>
>   at 
> org.eclipse.emf.transaction.impl.FilterManager.select(FilterManager.java:88) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl$1PrecommitRunnable.run(TransactionalEditingDomainImpl.java:607) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl.runExclusive(TransactionalEditingDomainImpl.java:289) 
>
>   at 
> org.eclipse.emf.transaction.util.TransactionUtil.runExclusive(TransactionUtil.java:327) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl$1PrecommitRunnable.runExclusive(TransactionalEditingDomainImpl.java:587) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl.precommit(TransactionalEditingDomainImpl.java:657) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionImpl.commit(TransactionImpl.java:353) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalCommandStackImpl.doExecute(TransactionalCommandStackImpl.java:70) 
>
>   at 
> org.eclipse.emf.transaction.impl.AbstractTransactionalCommandStack.execute(AbstractTransactionalCommandStack.java:165) 
>
> -----------
>
> java.lang.IndexOutOfBoundsException: toIndex = 2
>   at java.util.SubList.<init>(AbstractList.java:705)
>   at java.util.RandomAccessSubList.<init>(AbstractList.java:861)
>   at java.util.AbstractList.subList(AbstractList.java:570)
>   at 
> org.eclipse.emf.transaction.impl.ReadWriteValidatorImpl$NotificationTree.collectNotifications(ReadWriteValidatorImpl.java:410) 
>
>   at 
> org.eclipse.emf.transaction.impl.ReadWriteValidatorImpl$NotificationTree.collectNotifications(ReadWriteValidatorImpl.java:383) 
>
>   at 
> org.eclipse.emf.transaction.impl.ReadWriteValidatorImpl.getNotificationsForPostcommit(ReadWriteValidatorImpl.java:186) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl.postcommit(TransactionalEditingDomainImpl.java:717) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl.deactivate(TransactionalEditingDomainImpl.java:504) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionImpl.close(TransactionImpl.java:623) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionImpl.rollback(TransactionImpl.java:516) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionImpl.commit(TransactionImpl.java:357) 
>
>   at 
> org.eclipse.emf.transaction.impl.TransactionalCommandStackImpl.doExecute(TransactionalCommandStackImpl.java:70) 
>
>   at 
> org.eclipse.emf.transaction.impl.AbstractTransactionalCommandStack.execute(AbstractTransactionalCommandStack.java:165) 
>
> ------------------
>
>
> We have implemented the following workaround, but please let me know if 
> this is a bug or if you suggest any other solution.  Thanks!
>
>
> In TransactionChangeRecorder, we override the following method:
>
> @Override
> public void notifyChanged(final Notification notification) {
>       InternalTransactionalEditingDomain internalDomain = 
> getEditingDomain();
>       InternalTransaction activeTransaction = 
> internalDomain.getActiveTransaction();
>       if (activeTransaction != null) {
>           // There is already an active transaction
>           Thread owner = activeTransaction.getOwner();
>           Thread currentThread = Thread.currentThread();
>           if (owner != currentThread) {
>               TransactionUtils.writing(internalDomain, new Runnable() {
>                   public void run() {
>                       
> FixedTransactionChangeRecorder.super.notifyChanged(notification);
>                   }
>               });
>               return;
>           }
>       }
>       super.notifyChanged(notification);
> }