Comment 1 Karen Butzke 2008-05-27 10:25:17 EDT

*** Bug 228509 has been marked as a duplicate of this bug. ***

Comment 2 Karen Butzke 2008-05-27 17:49:47 EDT

The problem here is that our code is modifying the java and we are also listening for java event notifications.  Sometimes our changes and the java event notifications (on a different thread) will conflict and we will delete things out from under ourselves.  I'm not sure of the solution here, what we would really like is to be able to synchronously make changes in the jdt and be notified immediately.  That way we would know that we were the ones who had made the java change so we could ignore it in our updating.  This particular issue isn't usually a problem because a person can't use our JPA ui to add named queries as quickly as you can programatically.

We add the java listener in JpaModelManager with the static method JavaCore.addElementChangedListener(IElementChangeListener)

Comment 3 Neil Hauge 2009-08-03 12:11:47 EDT

I briefly hoped that the fix for bug 272752 might help with this problem, but I guess it depends on what the situation is with focus in the UI during programmatic update.  Any thoughts on this?

Comment 4 Kevan Holdaway 2009-12-16 15:48:45 EST

(In reply to comment #3)
> I briefly hoped that the fix for bug 272752 might help with this problem, but I
> guess it depends on what the situation is with focus in the UI during
> programmatic update.  Any thoughts on this?

I am seeing the exact problem described by Tom.  This problem hasn't been around for a while, but a recent change to Dali 2.3 has re-surfaced it.  We are using the following to get the events on the UI:

JpaProject jpaProject = entity.getPersistentType().getJpaProject();
			jpaProject.setThreadLocalModifySharedDocumentCommandExecutor(SynchronousUiCommandExecutor.instance());


The other curious thing is that when the entity class is open the problem does not occur.  This likely has something to do with the working copy from JDT.

This is causing our tooling some real problems.

Comment 5 Kevan Holdaway 2009-12-16 16:44:14 EST

One more note.  The stack trace is different that the one Tom saw, but this is expected given the amount of time that has passed.

Here is the stack trace I see.

java.lang.ArrayIndexOutOfBoundsException: Array index out of range: 2
	at java.util.Vector.elementAt(Vector.java:341)
	at java.util.Vector.get(Vector.java:462)
	at org.eclipse.jpt.core.internal.resource.java.source.SourceNamedQueriesAnnotation.moveNestedAnnotationInternal(SourceNamedQueriesAnnotation.java:102)
	at org.eclipse.jpt.core.internal.resource.java.source.SourceNamedQueriesAnnotation.moveNestedAnnotationInternal(SourceNamedQueriesAnnotation.java:1)
	at org.eclipse.jpt.core.internal.resource.java.source.AnnotationContainerTools.addNestedAnnotation(AnnotationContainerTools.java:46)
	at org.eclipse.jpt.core.internal.resource.java.source.SourcePersistentMember.addAnnotation(SourcePersistentMember.java:173)
	at org.eclipse.jpt.core.internal.jpa1.context.java.GenericJavaQueryContainer.addNamedQuery(GenericJavaQueryContainer.java:75)
	at org.eclipse.jpt.core.internal.jpa1.context.java.GenericJavaQueryContainer.addNamedQuery(GenericJavaQueryContainer.java:1)
	at com.ibm.etools.webtools.jpa.util.NamedQueriesUtil.addNamedQueriesOnEntity(NamedQueriesUtil.java:343)
	at com.ibm.etools.webtools.jpa.codegen.ManagerBeanCodeGenOperation.updateManagerBeanNamedQueries(ManagerBeanCodeGenOperation.java:356)
	at com.ibm.etools.webtools.jpa.codegen.ManagerBeanCodeGenOperation.execute(ManagerBeanCodeGenOperation.java:167)
	at com.ibm.etools.webtools.jpa.codegen.CodeGenOperation.run(CodeGenOperation.java:76)
	at com.ibm.etools.webtools.jpa.wizard.operations.JpaOperation.execute(JpaOperation.java:139)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl$1.run(DataModelPausibleOperationImpl.java:385)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1857)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl.runOperation(DataModelPausibleOperationImpl.java:410)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl.runOperation(DataModelPausibleOperationImpl.java:360)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl.doExecute(DataModelPausibleOperationImpl.java:247)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl.executeImpl(DataModelPausibleOperationImpl.java:219)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl.cacheThreadAndContinue(DataModelPausibleOperationImpl.java:89)
	at org.eclipse.wst.common.frameworks.internal.datamodel.DataModelPausibleOperationImpl.execute(DataModelPausibleOperationImpl.java:207)
	at org.eclipse.wst.common.frameworks.internal.datamodel.ui.DataModelWizard$1CatchThrowableRunnableWithProgress.run(DataModelWizard.java:224)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)

Comment 6 Kevan Holdaway 2010-02-04 16:52:54 EST

Any progress on this bug?  We really need this looked at.

Comment 7 David Williams 2010-02-04 19:20:11 EST

Kevan or Tom, I know its not easy, but can you provide a testcase/unit test on WTP that reproduces the problem? (I know that's not small request, but would sure make things easier). 

Karen, I don't know much about what you are doing ... but your comment about "...we would know that we were the ones who had
made the java change so we could ignore it in our updating" leads me to say a) I'm pretty positive there is no way to force a synchronous update :) and b) in other places, I've seen a technique used where "requested changes" were kept in a list, and removed when a matching event is reported back, so you'd know if you got an event you did not initiate came through, and required you to update. Not sure if it applies to your case ... but, thought I'd mention it. 

Kevan, I don't know about anyone else, but 

jpaProject.setThreadLocalModifySharedDocumentCommandExecutor(SynchronousUiCommandExecutor.instance());

is one of more frightening lines of code I've read it a while. (ha ha ha). That just shows how little I know of what you are doing.  So, I'll ask. What are you trying to do here? Put an operation on the UI thread so its executed on the UI thread? If so ... why is that? Normally a direct model change would not require it, so I'm just curious. Are there other problems you are trying to get around? 

thanks,

Comment 8 Dani Megert 2010-02-05 02:55:35 EST

>what we
>would really like is to be able to synchronously make changes in the jdt and be
>notified immediately.
What changes do you want to make? What exactly do you mean with synchronously?

Comment 9 Kevan Holdaway 2010-02-05 08:53:41 EST

We are modifying the named queries in the entities programmatically.  I unfortunately didn't write the code that calls the setThreadLocalModifySharedDocumentCommandExcecutor() method.  See below.

JpaProject jpaProject = entity.getJpaProject();
jpaProject.setThreadLocalModifySharedDocumentCommandExecutor(SynchronousUiCommandExecutor.instance());

It is my understanding that this code was added because Dali was missing events due to the fact it only listens to JDT changes on the UI thread.

The code that causes this problem basically removes named queries, updates queries, and then adds queries.  The named queries are modified using Dali API. They are modified in a JavaCore.run() batch operation.  According to the JDT java doc this runs the given operation as an atomic Java model operation.  Events are not sent until after the batch operation.  We use this because we are modifying many entities (and other java files) at the same time.  Eclipse suggests doing this when making large changes.

Below is some sample code of how we modifying the JPA named queries.  newValue, newName would be different for each iteration of the loop (we are not creating duplicates), but for simplicity I just use a simple variable.


//Remove all queries that were removed
for (NamedQuery q : queriesToRemove) {
    queryContainer.removeNamedQuery(q);
			}

//Update queries that have changed.
for (NamedQuery q : queriesToChange) {
   persistedQuery.setQuery(newValue);
}

//Add all queries that were new
for (JpaNamedQueryImpl q : queriesToAdd) {
    NamedQuery newNamedQuery = queryContainer.addNamedQuery(queryContainer.namedQueriesSize());
    newNamedQuery.setName(newName);
    newNamedQuery.setQuery(newValue);
}


Now, this problem is intermittent.  The array index out of bounds seems like its caused by adding the query to the end of the list. I use queryContainer.namedQueriesSize() to determine the end of the list.  So if there are 3 existing queries and I want to add it to the list the above code does a queryContainer.addNamedQuery(3) which would add the query to the 3rd position (on a 0 based system).

Somehow the query container's size doesn't match the actual number of named queries.  It usually fails after the first couple (e.g. on the 3rd or 4th).  So initially you add queries using the above code successfully, but after that the code gets out of sync.

A clear workaround is to just do queryContainer.addNamedQuery(0).  This will add it to the front of the list all the time.  However, its much more expensive because you always have to shift the entire list. Adding to the end would be ideal.

Comment 10 Dani Megert 2010-02-08 11:48:48 EST

To me this sounds like you simply need to make your query management more robust (thread-safe) and not try to get immediate change notifications from JDT.

Comment 11 Kevan Holdaway 2010-02-08 14:36:29 EST

(In reply to comment #10)
> To me this sounds like you simply need to make your query management more
> robust (thread-safe) and not try to get immediate change notifications from
> JDT.


I am not sure I follow.  Why would this command fail?  It was the QueryContainer Object that gave me the size and the QueryContainer object that failed to add.  How is this my code?

queryContainer.addNamedQuery(queryContainer.namedQueriesSize());

Comment 12 Kevan Holdaway 2010-02-08 14:38:18 EST

Here is the class that seems to have the issue:

org.eclipse.jpt.core.context.QueryContainer

Comment 13 Brian Vosburgh 2010-02-09 01:17:04 EST

I've been digging into this bug recently, and *maybe* I can answer a few of the questions.

Re comment 4 (and 5):
This problem may have re-surfaced when we changed how Dali handles events. In an effort to reduce deadlocks, we now handle events (e.g. IResourceChangeEvents, ElementChangedEvents) asynchronously. This will certainly change your timings, resulting in exposed race conditions (which these are). The race conditions are a result of Dali handling Java change events that were triggered by Dali itself changing the source code.

The reason you see this exception only when modifying entities that are *not* currently being edited is because of how Java change events are fired. If a Java source file is open in an editor, change events are fired asynchronously in a background thread (AbstractReconciler.BackgroundThread). This thread waits until there has been 0.5 seconds of inactivity before folding together the changes and firing the corresponding events. When a Java source file is *not* open in an editor, change events are fired synchronously when the changes are applied to the file.

Dali handles these Java change events asynchronously in yet another background thread. If you are programmatically changing an entity that is currently open in an editor, Dali will not receive any Java events until 0.5 seconds after all your changes have been applied. At this point, the Dali model and the Java source code are "in sync"; as a result, Dali will effectively ignore the incoming Java change event.

If you are programmatically changing an entity that is *not* currently open in an editor, Dali will receive any Java events immediately as your changes are being applied. Dali will dispatch these events to a background thread (in an effort to avoid those evil deadlocks). (Dali *used* to handle these events synchronously.) The background thread will be attempting to synchronize the Dali model with the recently "changed" Java source code. Unfortunately, both the Dali model and the Java source code will be changing continuously underneath the background thread as your client code continues on, adding named queries.

Re comment 7:
a) I'm also pretty sure there is no way to force a synchronous update. :-)

b) It would be enormously difficult for us to queue up requested changes to our model (e.g. add/remove/change/move named queries) and match them to asynchronous Java change events that fold together various text changes.

The code that frightens you so is the result of bug 184479. (It is *not* because Dali "only listens to JDT changes on the UI thread", as supposed in comment 9. :-) ) We added that hook because when we call TextEdit.apply(...) (which smells like a model object to me) the Document notifies a StyledText of a change and the StyledText tries to change its state, regardless of its current thread. Apparently, the Document must be changed on the UI thread? Or maybe we're just confused. (It's happened many times over the years, so I wouldn't be surprised....)

Re comment 8 (and 10):
Yes, we *do* need to make our model modifications more "thread-safe". It's not an insignificant task; since we are trying to keep it synchronized with Java and XML source code. Karen was simply airing our [pipe] dream that it would sure be nice if we received Java change notifications synchronously. Then it is entirely possible we would not have these race conditions. Of course, we've never seriously thought this would happen. :-) Anyway, that remark was based a pretty reasonable technique for synchronizing two models: Model A can ignore any change events from Model B that are the result of changes made *by* Model A *to* Model B. This will be obvious because Model A will already be in the "new" state indicated by the event when it comes back [synchronously].

Re comment 11:
I believe comment 10 was directed at Dali code, not your client code.

Re comment 9 (and 12):
The index out of range exception occurs because Dali's background thread is trying to synchronize the Dali model with the Java source code it has recently parsed, simultaneous with your code adding named queries to the model. My best guess at what happens:

1. Client code adds named query to the Dali model. This adds a named query annotation to the Java source code. This triggers a Java change event.

2. The Dali background thread receives the Java change event and parses the Java source code.

3. Client code adds another named query to the Dali model.

4. The background thread now comes through and sees there are two queries in the Dali model compared to the one query in the AST it created in step 2. It removes the extra query from the Dali model. (Whoops.)

5. Dali is still processing the client request from step 3 and assumes there are still two queries in the model. (Rats! Exception!)

Hopefully I've made sense with all this....

I'm pretty sure an easy solution is *not* immediately forthcoming; but I'm working on it....

Any insights are welcome.

Comment 14 Dani Megert 2010-02-09 02:32:02 EST

>Re comment 11:
>I believe comment 10 was directed at Dali code, not your client code.
Exactly. Thanks for the good explanation Brian.

> That way we would know that we were the ones who had
>made the java change so we could ignore it in our updating. 
Maybe you could detect that by checking in which thread the notification happens.

Comment 15 David Williams 2010-02-09 09:15:12 EST

Thanks for the nice summary, Brian, and letting us know work is in progress. 

I'm sure an "additional API" request will scare Dani away 
from following this bug :)  but in some cases in the past, in other areas, we've used a "requester" object field on "change requests", usually set to "this" by the requester, and it would just be passed along and become "originator" on the eventual notification events. Requesters could then use that field to know if "they" were the ones that started the whole thing. (Since this would be a recent addition, it would often be null, for old code where requests were made without the notifier, so that would have to remain to be ok, for compatibility). 

Perhaps Dani's thread identification would work as well with no additional API ... just thought I'd throw it out there and see what sticks.

Comment 16 Brian Vosburgh 2010-02-09 11:28:00 EST

(In reply to comment #14)
> > That way we would know that we were the ones who had
> >made the java change so we could ignore it in our updating. 
> Maybe you could detect that by checking in which thread the notification
> happens.

I'm not sure I understand your suggestion. Java change notifications can arrive either on the "Dali" thread (when Dali is changing a non-edited Java file) or on the JavaReconciler thread (when Dali is changing a Java file currently being edited). Wouldn't checking the thread still require synchronous notification?

Comment 17 Dani Megert 2010-02-09 11:38:20 EST

Right, but you might be able to use that information together with other info to finally detect where the change started. I'm just throwing in breadcrumbs here ;-)

Synchronizing the Java deltas is not a good idea because that can result in UI (e.g. Outline) lacking behind or even in a blocked UI.

Comment 18 Brian Vosburgh 2010-02-09 11:45:31 EST

(In reply to comment #15)
> "requester" API
This sounds sorta tough: tracking the clients making changes to a text file and folding their changes together over time. But maybe it's easy. :-) I don't know.

Comment 19 Brian Vosburgh 2010-02-09 11:48:47 EST

(In reply to comment #17)
> Right, but you might be able to use that information together with other info
> to finally detect where the change started. I'm just throwing in breadcrumbs
> here ;-)
> 
> Synchronizing the Java deltas is not a good idea because that can result in UI
> (e.g. Outline) lacking behind or even in a blocked UI.

Yeah, I know. Like I said, "pipe dream". :-) Anyway, thanks for the breadcrumbs. Although - I've been living on breadcrumbs related to these problems for several years now; you would think I would've lost some weight by now. :-)

Comment 20 Dani Megert 2010-02-09 11:50:23 EST

Such an API will only exist in your dreams ;-)

Comment 21 Kevan Holdaway 2010-02-16 15:07:26 EST

What would really help me is to find a work around.  In comment 9, I indicated that I could work around this by simply adding to the front of the list.  It turns out that this will not work either (using queryContainer.addNamedQuery(0)).  It appears that doing this will drop the first query.

So here is what I am seeing.  If I add 5 queries, only the last 4 queries appear.  The first query gets silently dropped.

What I really need is either a fix or some idea how to work around this.

Comment 22 Billy Rowe 2010-02-16 17:30:20 EST

Kevan is seeing ArrayIndexOutOfBoundsException even when adding to the front of the array. Could we get an update on when a fix for this bugzilla is anticipated?

Comment 23 Brian Vosburgh 2010-02-18 15:43:48 EST

I have been working on this bug for a while now and will continue to work on it. We are hoping to fix this bug for M6; but that is being aggressive, given how much work is necessary.

One possible workaround is using the Dali resource model API to add your queries. Look at the code in GenericJavaQueryContainer.addNamedQuery(int) to see how to add a query annotation to your source code. You can then set the name and query of the returned query annotation. You can get hold of the JavaResourcePersistentType via JavaPersistentType.getResourcePersistentType(). Once you have added all your queries, you can call JpaProject.synchronizeContextModel(). This will refresh the context model to be in sync with the resource model.

Comment 24 Brian Vosburgh 2010-03-02 09:57:18 EST

Please let us know if the workaround suggested in comment 23 can work, as the fix for this bug is proving too big and risky to make it into Helios.

Comment 25 Kevan Holdaway 2010-03-02 10:53:33 EST

(In reply to comment #24)
> Please let us know if the workaround suggested in comment 23 can work, as the
> fix for this bug is proving too big and risky to make it into Helios.

No, it didn't.  I couldn't find a straight forward way to do this.  I could just use JDT to add annotations to a java class, but I think this bug should remain open to address thread safety for Dali.

Comment 26 Neil Hauge 2010-03-03 17:03:13 EST

This bug will certainly remain open, as we will continue to pursue this work.  We can provide more assistance with the workaround if you are interested in using the Dali resource model API to do this.  This should be an easier path than working with the AST, but it sounds like you ran into some problems?

This bug is a manifestation of a much larger issue (see comments above), and addressing that issue will take a large refactoring effort as Brian has mentioned.  Given the work effort remaining, and the risk associated with that work, our thinking is this change would be better suited for an an early milestone for the next release as opposed to going into Helios late.  As a result I am targeting this bug our 3.0 release for now.

Comment 27 Raghunathan Srinivasan 2010-12-09 20:02:12 EST

I get this exception while typing the query in the source view. I did invoke content assist and quick fix at multiple points while creating the query. I did notice one extra query in the property editor which was a duplicate of the last query.


Exception from the log file:
java.lang.IndexOutOfBoundsException: Index: 2, Size: 2
at org.eclipse.jpt.utility.internal.model.value.CompositeListValueModel.get(CompositeListValueModel.java:191)
at org.eclipse.jpt.utility.internal.model.value.ItemAspectListValueModelAdapter.get(ItemAspectListValueModelAdapter.java:92)
at org.eclipse.jpt.ui.internal.swt.TableModelAdapter.selectedItems(TableModelAdapter.java:600)
at org.eclipse.jpt.ui.internal.swt.TableModelAdapter.tableSelectionChanged(TableModelAdapter.java:586)
at org.eclipse.jpt.ui.internal.swt.TableModelAdapter$3.widgetSelected(TableModelAdapter.java:260)
at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:234)
at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4066)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3657)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2640)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2604)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2438)
at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:671)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:664)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:369)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.lang.reflect.Method.invoke(Unknown Source)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:619)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:574)
at org.eclipse.equinox.launcher.Main.run(Main.java:1407)

Comment 28 Neil Hauge 2011-02-01 17:34:11 EST

A change has been made to address this issue in M5, but we're not quite ready to mark this as fixed yet.  Adopter testing would be helpful to ensure this issue has been resolved.  I'm moving the target to M6 so this bug doesn't look invalid, but the target may be put back to M5 if the fix is confirmed.

Comment 29 Kenneth Cheung 2011-02-03 14:48:43 EST

Any chance it can be backported to Eclipse 3.6?  Our adoptor product is on 3.6 and really need this fixed.  Thanks.

Comment 30 Brian Vosburgh 2011-02-03 15:00:29 EST

No, this change will not be backported. It affects hundreds of classes and changes API.

Comment 31 Neil Hauge 2011-04-26 23:06:40 EDT

Given that we haven't gotten any negative feedback from clients (indicating that this change has not fixed the problem) and our own testing has not revealed any issues, I am closing this as fixed for now.  Please let us know if you are still seeing problems of this nature and we will investigate and reopen this bug if necessary or open new bugs if appropriate.