Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 371091 - FlowFigure#add(IFigure child, Object constraint, int index) can causes NPE because FlowContext is set too late.
Summary: FlowFigure#add(IFigure child, Object constraint, int index) can causes NPE be...
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: 3.7.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-09 10:10 EST by François POYER CLA
Modified: 2013-01-05 04:13 EST (History)
1 user (show)

See Also:


Attachments
This is a patch file that implements fix as proposed by the submitter (2.70 KB, patch)
2013-01-05 04:13 EST, Willem Duminy CLA
willem.duminy: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description François POYER CLA 2012-02-09 10:10:43 EST
In FlowFigure:
<code>
	public void add(IFigure child, Object constraint, int index) {
		super.add(child, constraint, index);
		// If this layout manager is a FlowContext, then the child *must* be a
		// FlowFigure
		if (getLayoutManager() instanceof FlowContext)
			((FlowFigure) child)
					.setFlowContext((FlowContext) getLayoutManager());
		revalidateBidi(this);
	}
</code>

Problem is on first line: "super.add(child, constraint, index);" causes (in Figure#add) a call to #revalidate(), which signals the figure as invalid to the UpdateManager, while the child Figure doesn't have its FlowContext set yet (and so will cause a NPE if UpdateManager tries to validate it right away).

Setting the FlowContext of child figure before calling super.add does fix the problem.

Note: For some reasons specific to my project, I do not use the "standard" DeferredUpdateManager that queues validation work until a PAINT event occurs but a more "synchronous" one that performs validation as soon as invalid figures are signaled.

<code>
public synchronized void addInvalidFigure(final IFigure f) {
    // Note the invalid figure...
    this.invalidFigures.add(f);
    // ...and perform update and validation right away!
    Display display = Display.getCurrent();
    if (display == null) {
        throw new SWTException(SWT.ERROR_THREAD_INVALID_ACCESS);
    }
    display.syncExec(new UpdateRequest());
}
</code>

This is why the problem occurs every time for me.
But the problem might as well happen with the regular DeferredUpdateManager, depending on multithread weaving I guess. 
More importantly: FlowFigure implementation should probably not depend on some assertion about the UpdateManager's implementation, should it?
Comment 1 Willem Duminy CLA 2013-01-05 04:13:52 EST
Created attachment 225240 [details]
This is a patch file that implements fix as proposed by the submitter

Seems like a reasonable fix for the potential problem.