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

Bug 415735

Summary:	EHandlerService does not always return the correct result if called on a non UI Thread
Product:	[Eclipse Project] Platform	Reporter:	Thorsten Hake <thorsten.hake>
Component:	UI	Assignee:	Platform UI Triaged <platform-ui-triaged>
Status:	CLOSED WONTFIX	QA Contact:
Severity:	normal
Priority:	P3	CC:	pwebster
Version:	4.3	Keywords:	helpwanted
Target Milestone:	---
Hardware:	PC
OS:	Windows 7
Whiteboard:	stalebug

Description Thorsten Hake

2013-08-23 03:49:02 EDT

The EHandlerService internally uses a LinkedList (contextStack) to manage the ExecutionContexts that have to be used in order to evaluate if a Handler can be executed or not.

Unfortunately the contextStack is not thread safe. So the following condition may happen:
Non-UI Thread:
- Checks if a Command can be executed in a certain context (calling canExecute(ParameterizedCommand, IEclipseContext)
- EHandlerService adds the given context to the first position of the contextStack

Meanwhile in the UI-Thread:
- Constantly checks if a Command can be executed in its context (HandledContributionItem)
- Therefore canExecute is called on the EHandlerService.
- EHandlerService adds the given context at the first position of the contextStack

Meanwhile in the Non-UI-Thread:
- Checks if the Command can be executed by peeking at the first element of the contextStack.
- Evaluation of @CanExecute will be done using the IEclipseContext pushed by the UI-Thread, not the IEclipseContext that has been used to call canExecute.

Thus, if the EHandlerService is used in multiple threads, the method contract of canExecute(ParameterizedCommand, IEclipseContext) and executeHandler(ParameterizedCommand, IEclipseContext) are broken. The calls to the methods annotated with @CanExecute and @Execute are not always performed in the passed IEclipseContext.

Comment 1 Paul Webster

2013-08-23 08:08:53 EDT

It wasn't designed to be thread safe, as in 3.x it's supposed called from the UI thread.

In addition, I'm not sure that this pattern can be made thread safe, as there's a couple of back and forth steps between the HandlerServiceImpl, the HandlerServiceHandler, and the actual invoked handler.

PW

Comment 2 Thorsten Hake

2013-08-23 08:19:33 EDT

Wouldn't it be sufficient to make the contextStack ThreadLocal?

To not create a memory leak, one could dynamically remove the contextStack variable for a thread if the contextStack is empty. This of course leads to dynamic initialization.

I will try to provide a patch if this way to fix it is accaptable.

Comment 3 Lars Vogel

2019-11-27 07:15:01 EST

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.

If the bug is still relevant, please remove the stalebug whiteboard tag.