Community
Participate
Working Groups
The Binding subclasses handle observable change events in their doUpdate method. The method is called inside the source observable's realm and executes a runnable in the destination's realm. The Binding class also listens for the dipose event of both observables and handles such an event in the validation realm. Inside that realm it removes the change listener from both observables to avoid further events. But if one change event is currently pending while the destination observable gets disposed, the Runnable inside doUpdate while cause an AssertionFailedException because of the disposed Observable. This is no theoretical problem, but it happens in my company. The patch coming shortly will contain the fix and a test that verifies the correct behavior (and also fails without the changes).
After more investigation the problem only seems to happen on the WritableSet, as the add method calls getterCalled() which checks the disposed state. In the WritableValue and WritableList classes the modifying operations (setValue, add, remove...) don't call getterCalled but only checkRealm. So they don't fail with an exception if a modification happens after disposing. After looking into the GIT history I see that the additional getterCalled checks in WritableSet were introduced in 2008 for bug 221351. The bug title also includes the List, so I don't know why the additional checks were only introduced for the Set. My idea is to protect all Binding subclasses against disposed observables although List and Value currently don't have a problem.
New Gerrit change created: https://git.eclipse.org/r/119156
Gerrit change https://git.eclipse.org/r/119156 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=1637c214b12705a4783fcf9593addfda3230a99f
Thanks, Conrad!
New Gerrit change created: https://git.eclipse.org/r/122362
Gerrit change https://git.eclipse.org/r/122362 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=680f8a69eed99d964d026c53a5ec257e24f3354b
Changes were requested and PMC approval was missing
New Gerrit change created: https://git.eclipse.org/r/148775
I plan to merge the changes to the databinding in https://git.eclipse.org/r/#/c/148775/, but without the changes to ThreadRealm. Can anyone explain the purpose of the changes to TheadRealm? If so we can make those changes in a separate commit.
(In reply to Jens Lideström from comment #9) > I plan to merge the changes to the databinding in > https://git.eclipse.org/r/#/c/148775/, but without the changes to > ThreadRealm. > > Can anyone explain the purpose of the changes to TheadRealm? If so we can > make those changes in a separate commit. Without the changes in ThreadRealm the added test-cases are not failing without this change.
(In reply to Conrad Groth from comment #10) > Without the changes in ThreadRealm the added test-cases are not failing > without this change. Thanks for the info, Conrad! I'll add the changes to ThreadRealm in their own commit. Now to try to find out why the new test is currently failing...
Gerrit change https://git.eclipse.org/r/148775 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=3f95ca7b80f48fb2688405a76b1f4cd2503ee8cc
I finally came around to doing this. Sorry about the delay. Previously I ran in to some weird problems with the test, which probably only was me being confused. When I run the tests now everything works as expected: * WITHOUT the fix in Binding and WITHOUT the changes to ThreadRealm the new test succeeds. * WITHOUT the fix in Binding but WITH the changes to ThreadRealm the new test fails. * WITH the fix in Binding and WITH the changes to ThreadRealm the new test succeeds. Thanks Conrad for the fix, and especially for new test and the tricky updates to ThreadRealm!
