Skip to main content


Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » Eclipse 4 » TranslationObjectSupplier seems to leak FieldRequestor?
TranslationObjectSupplier seems to leak FieldRequestor? [message #1755932] Thu, 09 March 2017 11:29 Go to next message
Christian Mohr is currently offline Christian MohrFriend
Messages: 34
Registered: June 2012
Member
Hi,

while investigating some internal memory leaks we stumpled upon the TranslationObjetSupplier. It looks like it never actually removes FieldRequestor from its internal ListenerMap, even when the Class-Instances containing the @Translation annotation where already removed.

I tried to reproduce it in a simple application. I used the Eclipse E4-Template with Sample Classes and replaced the generated classes with the following ones:

The CustomComposites Class using the @Translation Annotation
public class CustomComposite extends Composite {
	@Inject
	@Translation
	public Messages messages;

	/**
	 * not used here for testshowcase, but needed in actual implementation
	 */
	@Inject
	public IEclipseContext context;

	@Inject
	public CustomComposite(final Composite parent) {
		super(parent, SWT.NONE);
		setLayout(new GridLayout(2, false));
		addDisposeListener(e -> destroy());
	}

	@PostConstruct
	public void postConstruct() {
		Label label = new Label(this, SWT.NONE);
		label.setText(messages.myFirstLabel);
		label = new Label(this, SWT.NONE);
		label.setText(messages.mySecondLabel);
	}

	public void destroy() {
		context.dispose();
	}
}


The SamplePart for adding and removing "CustomComposites"
public class SamplePart {

	List<CustomComposite> customComposites = new ArrayList<>();

	@Inject
	private IEclipseContext context;

	@PostConstruct
	public void createComposite(final Composite parent) {
		parent.setLayout(new GridLayout(2, false));
		Button buttonAdd = new Button(parent, SWT.PUSH);
		buttonAdd.setText("ADD");
		buttonAdd.addSelectionListener(new SelectionAdapter() {
			@Override
			public void widgetSelected(final SelectionEvent e) {
				IEclipseContext childContext = context.createChild("myChildContexts");
				childContext.set(Composite.class, parent);
				CustomComposite make = ContextInjectionFactory.make(CustomComposite.class, childContext);
				customComposites.add(make);
				make.requestLayout();
			}
		});
		Button buttonRemove = new Button(parent, SWT.PUSH);
		buttonRemove.setText("REMOVE");
		buttonRemove.addSelectionListener(new SelectionAdapter() {
			@Override
			public void widgetSelected(final SelectionEvent e) {
				if (!customComposites.isEmpty()) {
					CustomComposite composite = customComposites.get(customComposites.size() - 1);
					customComposites.remove(composite);
					composite.dispose();
					parent.requestLayout();
				}
			}
		});
	}
}


And the Messages class (+ the translation file):
public class Messages {
	public String myFirstLabel;
	public String mySecondLabel;
}


While debugging you can see, that in the TranslationObjectSupplier, for each added CustomComposite, there is an additional FieldRequestor added to the internal list (addListener method). After disposing the CustomComposites, they won't get removed. With any Memory-Analyzer you can see that the amount of FieldRequestor- and IEclipseContext-Instances is growing.

Finally my question is whether this is a real bug, and i should open a bug report for it, or am i doing something completly wrong with the Annotation or EclipseContext-Handling.

Thanks for any help.

Regards,
Christian

[Updated on: Thu, 09 March 2017 11:38]

Report message to a moderator

Re: TranslationObjectSupplier seems to leak FieldRequestor? [message #1755937 is a reply to message #1755932] Thu, 09 March 2017 12:34 Go to previous messageGo to next message
Dirk Fauth is currently offline Dirk FauthFriend
Messages: 2602
Registered: July 2012
Senior Member
Looking at the current implementation the IRequestor only get removed when you change the locale and the IRequestor became invalid. If you never change the locale the reference will never be removed.

So yes it looks like a memory leak here. Please create a ticket for this, so we can discuss how this can be solved. As far as I know there is no listener mechanism that would get informed.
Re: TranslationObjectSupplier seems to leak FieldRequestor? [message #1755949 is a reply to message #1755937] Thu, 09 March 2017 13:32 Go to previous messageGo to next message
Christian Mohr is currently offline Christian MohrFriend
Messages: 34
Registered: June 2012
Member
Thanks Dirk for your response,

The ticket: https://bugs.eclipse.org/bugs/show_bug.cgi?id=513377

My current workaround is to put the @Translation injection into the constructor. But for other appliations this may have some limitations.


Re: TranslationObjectSupplier seems to leak FieldRequestor? [message #1755958 is a reply to message #1755949] Thu, 09 March 2017 15:07 Go to previous messageGo to next message
Dirk Fauth is currently offline Dirk FauthFriend
Messages: 2602
Registered: July 2012
Senior Member
Thanks.

Could you try to make use of the BaseMessageRegistry to see if that would solve the leakage?

For this you create a new message registry class like this:
@Creatable
public class MessageRegistry extends BaseMessageRegistry<Messages> {

	@Override
	@Inject
	public void updateMessages(@Translation Messages messages) {
		super.updateMessages(messages);
	}
}


and use it in your composite like this:

public class CustomComposite extends Composite {
	
	/**
	 * not used here for testshowcase, but needed in actual implementation
	 */
	@Inject
	public IEclipseContext context;

	@Inject
	public CustomComposite(final Composite parent) {
		super(parent, SWT.NONE);
		setLayout(new GridLayout(2, false));
		addDisposeListener(e -> destroy());
	}

	@PostConstruct
	public void postConstruct(MessageRegistry registry) {
		Label label = new Label(this, SWT.NONE);
		registry.register(label::setText, m -> m.myFirstLabel);
		label = new Label(this, SWT.NONE);
		registry.register(label::setText, m -> m.mySecondLabel);
	}

	public void destroy() {
		context.dispose();
	}
}


As the references in the TranslationObjectSupplier are never cleaned up, I suspect the same behavior, I just want to be sure.
Re: TranslationObjectSupplier seems to leak FieldRequestor? [message #1755966 is a reply to message #1755958] Thu, 09 March 2017 15:36 Go to previous messageGo to next message
Christian Mohr is currently offline Christian MohrFriend
Messages: 34
Registered: June 2012
Member
It is the same behavior. References added for each MessageRegistry to TranslationObjectSupplier. Instead of FieldRequestor it is now MethodRequestor.
Re: TranslationObjectSupplier seems to leak FieldRequestor? [message #1756085 is a reply to message #1755966] Fri, 10 March 2017 17:32 Go to previous messageGo to next message
Christian Mohr is currently offline Christian MohrFriend
Messages: 34
Registered: June 2012
Member
While looking for possible solution, i saw that the PreferenceObjectSupplier is also adding FieldRequestors and MethodRequestors into an internal map. These are also not removed.

Would it be possible to extend the ExtendedObjectSupplier with a "remove" method which is called when the Requestor is disposed? Or is there maybe another approach?
Re: TranslationObjectSupplier seems to leak FieldRequestor? [message #1756104 is a reply to message #1756085] Sat, 11 March 2017 11:07 Go to previous message
Dirk Fauth is currently offline Dirk FauthFriend
Messages: 2602
Registered: July 2012
Senior Member
Not sure at the moment. Could we move the discussion to the ticket you created? That might raise more visibility and response. Wink
Previous Topic:Using e4view still requires class to be IViewPart
Next Topic:Exception in Application start method
Goto Forum:
  


Current Time: Mon Jun 18 21:11:05 GMT 2018

Powered by FUDForum. Page generated in 0.02276 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software

Back to the top