Community
Participate
Working Groups
Using org.eclipse.core.databinding_1.3.100 There's a conceptual error in MultiValidator#revalidate() which leads to a dependency loop. Let's assume that ObservableTracker returns the following dependencies: dep[0] = observable1; dep[1] = observable2; dep[2] = targets; // same instance as the field target in this class MultiValidator The field "targets" holds: targets[0] = observable1; targets[1] = observable2; Because the outer loop iterates over targets, the invalid dependency to the field "targets" is not removed from newTargets and then added. This causes the endless loop in subsequent calls. Stacktrace snippet: at org.eclipse.core.databinding.observable.ChangeManager.fireEvent(ChangeManager.java:119) at org.eclipse.core.databinding.observable.AbstractObservable.fireChange(AbstractObservable.java:65) at org.eclipse.core.databinding.observable.list.ObservableList.fireListChange(ObservableList.java:72) at org.eclipse.core.databinding.observable.list.WritableList.addAll(WritableList.java:191) at org.eclipse.core.databinding.validation.MultiValidator.revalidate(MultiValidator.java:295) at org.eclipse.core.databinding.validation.MultiValidator$DependencyListener.handleChange(MultiValidator.java:152) at org.eclipse.core.databinding.observable.ChangeEvent.dispatch(ChangeEvent.java:41) at org.eclipse.core.databinding.observable.ChangeManager.fireEvent(ChangeManager.java:119)
*** Bug 332505 has been marked as a duplicate of this bug. ***
Created attachment 185119 [details] Proposed Patch Iterate twice over the newTargets. First remove all dependencies that would lead to a dependency loop, second compare with existing dependencies in target list.
Stefan, thanks for the patch! Would you mind doing the following changes to your patch in order to see whether we can apply it: * include a contribution note to the header of all the files you touched (including your name and the bug number as in the existing ones) * please move the (existing) comment "This loop is roughly equivalent to..." in the patch from the first to the second loop where it fits better * please try to write a test in MultiValidatorTest reproducing the stack overflow which should be fixed by your patch BTW, if you need a workaround for the bug, you might try accessing the targets observable inside the validate method protected by a "ObservableTracker.setIgnore(true);", something like the following: ObservableTracker.setIgnore(true); try { // observables accessed here are not tracked as dependencies } finally { ObservableTracker.setIgnore(false); }
Created attachment 185295 [details] Improved patch Ovidio, I changed point 1 and 2 according to your suggestions. Unfortunately, I didn't manage to implement a test case that determistically fails. The reason is that the dependency array returned by the ObservableTracker is based on a HashSet. As the order is crucial (as described in first comment) all my attempts only sometimes fail - which is obviously bad. Maybe you have an idea for a predictable test case?
Stefan, indeed it's probably hard to make the test causing a stack overflow deterministic since ObservableTracker uses an IdentitySet which uses System.identityHashCode(Object) which can change on each run of the test. However, you might test the root cause of the problem which is the fact that the internal observables end up in the "targets" list. You might write a test which accesses all of the internal observables inside the validate() method and make sure that none of them ends up in the "targets" list.
I think you hit the nail on the head: we filter out the new dependencies while iterating over the old ones, we can get internal observables past the filter by referencing them where there are no dependencies. I'm working on a test case for this.
Created attachment 203567 [details] Patch with tests This patch includes Stefan's fix plus a unit test for it. It also includes a fix plus test for an infinite loop in the getValidationStatus lazy init section. Stefan, can you test whether this patch addresses your problem?
Created attachment 203568 [details] mylyn/context/zip
comment 7: s/infinite loop/infinite recursion/
Ovidio, +1?
+1
Released to HEAD > 20110919
This should have been committed to git. Reopening.
Committed changeset c5f5b7d8201363f4bbe7cdb60f8de9ce51709d13 to R3_maintenance > 2011-09-23
Just for the records: the bundle version was not set correctly: it should have been 1.4.100.
