|
Re: EMF Transaction: Notification list will not be safely modified [message #423950 is a reply to message #423918] |
Fri, 10 October 2008 13:32 |
Eclipse User |
|
|
|
Originally posted by: cdamus.zeligsoft.com
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(Not ificationFilter.java:65)
>
> at
> org.eclipse.emf.transaction.impl.FilterManager.select(Filter Manager.java:88)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainI mpl$1PrecommitRunnable.run(TransactionalEditingDomainImpl.ja va:607)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainI mpl.runExclusive(TransactionalEditingDomainImpl.java:289)
>
> at
> org.eclipse.emf.transaction.util.TransactionUtil.runExclusiv e(TransactionUtil.java:327)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainI mpl$1PrecommitRunnable.runExclusive(TransactionalEditingDoma inImpl.java:587)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainI mpl.precommit(TransactionalEditingDomainImpl.java:657)
>
> at
> org.eclipse.emf.transaction.impl.TransactionImpl.commit(Tran sactionImpl.java:353)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalCommandStackIm pl.doExecute(TransactionalCommandStackImpl.java:70)
>
> at
> org.eclipse.emf.transaction.impl.AbstractTransactionalComman dStack.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$Noti ficationTree.collectNotifications(ReadWriteValidatorImpl.jav a:410)
>
> at
> org.eclipse.emf.transaction.impl.ReadWriteValidatorImpl$Noti ficationTree.collectNotifications(ReadWriteValidatorImpl.jav a:383)
>
> at
> org.eclipse.emf.transaction.impl.ReadWriteValidatorImpl.getN otificationsForPostcommit(ReadWriteValidatorImpl.java:186)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainI mpl.postcommit(TransactionalEditingDomainImpl.java:717)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalEditingDomainI mpl.deactivate(TransactionalEditingDomainImpl.java:504)
>
> at
> org.eclipse.emf.transaction.impl.TransactionImpl.close(Trans actionImpl.java:623)
>
> at
> org.eclipse.emf.transaction.impl.TransactionImpl.rollback(Tr ansactionImpl.java:516)
>
> at
> org.eclipse.emf.transaction.impl.TransactionImpl.commit(Tran sactionImpl.java:357)
>
> at
> org.eclipse.emf.transaction.impl.TransactionalCommandStackIm pl.doExecute(TransactionalCommandStackImpl.java:70)
>
> at
> org.eclipse.emf.transaction.impl.AbstractTransactionalComman dStack.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(notificat ion);
> }
> });
> return;
> }
> }
> super.notifyChanged(notification);
> }
>
|
|
|
|
Re: [EMF Transaction] Notification list will not be safely modified [message #423958 is a reply to message #423956] |
Fri, 10 October 2008 19:49 |
Eclipse User |
|
|
|
Originally posted by: cdamus.zeligsoft.com
Hi, Ben,
I would certainly be open to the IllegalStateException in the case of
notifications without a transaction context while another transaction is
in progress on another thread. This is consistent with the case where
there is no transaction at all.
By any chance, is the offending notification in your scenario one that
is compatible with a read-only context, such as a proxy resolution? The
reason I ask is that, currently, there is no assertion that these must
occur in a transaction context of any kind because we don't have a
mechanism to assert transactions in the general case of reading the
model content.
Please, do open a bugzilla. We can explore further the options and
their feasibility, there. I expect that *something* will need to be done.
Thanks,
Christian
Ben wrote:
> Hi Christian,
>
> Your analysis is correct. The TransactionUtils.writing method is as you
> suggested. We put in this workaround as an expedient solution, so you
> may have noticed the race condition after (activeTransaction != null)
> and before the notification gets recorded.
>
> Another reasonable approach would be to throw an IllegalStateException
> (or perhaps a ConcurrentModificationException?) when a notification
> comes in while a transaction is in play. This would be similar to the
> way that the IllegalStateException is thrown if the someone tries to
> open a read-write transaction while in a read-transaction. What do you
> think? Also, please let me know if you want me to open a bugzilla for
> this?
>
> Thanks!
>
>
>
|
|
|
|
Powered by
FUDForum. Page generated in 0.03072 seconds