Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 344033

Summary:

RATs can end up with incorrect nodes

Product:

[Eclipse Project] e4

Reporter:

Eric Moffatt <emoffatt>

Component:

Assignee:

Project Inbox <e4.ui-inbox>

Status:

RESOLVED WORKSFORME

QA Contact:

Severity:

normal

Priority:

CC:

remy.suen

Version:

unspecified

Target Milestone:

---

Hardware:

OS:

Windows XP

Whiteboard:

stalebug

Attachments:

Description	Flags
The loop of a stack overflow in RAD caused by this defect	none
Patch to issue the motifications through UISynchronize.asynchExec	none
ESelectionServiceTest patch v1	none

Description Eric Moffatt

2011-04-27 15:30:56 EDT

We found this while trying to get RAD running on top of 4.1...

The initial indication that something was wrong was that we were getting a stack overflow (I'll attach the actual loop trace).

After a bit of skull-scratching we determined that the root cause was that we were using a RAT to handle selection change notifications...bad idea...

The problem is that by calling out to client code while still inside the RAT *ANY* changes to the context caused by any client code (through calls to 3.x API...) get tagged by the RAT, causing the RAT to fire whenever that context node changes.

In our case it was the SelectionService 'track' RAT that was causing the issue but there could be others.

Note that this was never intended to be called (ever) on anything except a change in 'output.selection' but because we allow client code to run the changes that could take place during its execution are essentially unbounded (i.e. anything could happen..;-). The end result in this case was a stack overflow so at least we knew we had an issue but more insidious would be that we would end up spamming 'selection changes' under situations where the selection has not changed.

While we may be able to code around the particular cases we can identify perhaps some more general solution could be offered within the DI infrastructure (i.e. a way to disable 'tracking' from within a RAT...).

Comment 1 Eric Moffatt

2011-04-27 15:32:46 EDT

Created attachment 194198 [details]
The loop of a stack overflow in RAD caused by this defect

Comment 2 Eric Moffatt

2011-04-27 15:35:26 EDT

Note that we managed to code around this by running the notification loop in an 'asynchExec' (so that it was not running inside the RAT). While my initial code used Display.asynchExec it would likely be better to use the one from Realm to avoid YASD (Yet Another SWT Dependency)...

Comment 3 Eric Moffatt

2011-04-28 14:41:12 EDT

Created attachment 194301 [details]
Patch to issue the motifications through UISynchronize.asynchExec


This prevents the leakage since the actual notifications are no longer issued from within the RAT.

Comment 4 Eric Moffatt

2011-04-28 14:42:22 EDT

Committed in >20110428. Applied the patch.

Comment 5 Eric Moffatt

2011-04-28 14:45:07 EDT

The previous fix is probably sufficient for 4.1. If other instances of this anti-pattern are discovered they should also be repaired using the same technique.

I'll leave this defect open to see if there is a possible solution on the DI side which would allow us to safely perform client code from inside a RAT synchronously.

Comment 6 Remy Suen

2011-04-28 14:57:08 EDT

Created attachment 194305 [details]
ESelectionServiceTest patch v1

(In reply to comment #3)
> Created attachment 194301 [details]
> Patch to issue the motifications through UISynchronize.asynchExec

The fix above has caused all the tests in the ESelectionServiceTest test class to fail.

Comment 7 Remy Suen

2011-04-28 14:57:52 EDT

(In reply to comment #6)
> Created attachment 194305 [details]
> ESelectionServiceTest patch v1

Patch released to CVS HEAD.

Comment 8 Eclipse Genie

2018-10-24 16:20:22 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.

Comment 9 Lars Vogel

2019-06-05 07:36:16 EDT

This is a mass change to close all e4 bugs marked with "stalebug" whiteboard.

If this bug is still valid, please reopen and remove the "stalebug" keyword.