Community
Participate
Working Groups
> copied from email conversation: SWTException, SWTError and IllegalArgument Exception are sent when there are usage errors in the SWT APIs, so they should be filtered and sent to the client project using the SWT APIs. The list also contains IllegalStateException that is unrelated to SWT. From the list, the following Exceptions seem useful: 1. NPE 2. AIOOB 3. StringIndexOutOfBoundsException 4. ArithmeticException
Can you please confirm that SWTError is actually a client exception? I remember "No more handles" exceptions use SWTError and I don't think that this is caused by a client/caller of SWT. But my understanding may be wrong.
Please review [1]. It's an IAE but it occurs deeply inside an SWT stack. Until Display.readAndDispatch there is even no other project involved. To me this seems like a problem in SWT rather than in any of SWT's clients. I'll adjust the project guessing so that in those cases - if only SWT is on the stack and it's a SWT client exception type - the problem will (still) be associated with SWT. [1] https://dev.eclipse.org/recommenders/committers/confess/#/problems/55d9bd89e4b0f0b83a6ec4dc/details
(In reply to Marcel Bruch from comment #2) I need your advice. There are corner cases I don't know how to handle properly yet. [1], for instance, is an IAE which is a client's fault. But I've been teached that every frame below Display.readAndDispatch *may or may not* be related to the exception above and I should cut off the analysis after that frame. Is this general rule wrong? For [1] it looks like. But is there a general pattern how I can reliably assign this problem to the right project? And: What is the right project? PartRenderingEngine$4.run is causing the problem but likely on behalf of one of its clients which cannot be found on the stack trace. It looks like we need a better logging in those cases. https://dev.eclipse.org/recommenders/committers/confess/#/problems/55d9bd89e4b0f0b83a6ec4dc/details
"org.eclipse.swt.SWTException: Return value not valid" in https://dev.eclipse.org/recommenders/committers/confess/#/problems/559ba76ce4b079cbdd2cbd84/details also does not look like a SWT client's fault. It looks like org.eclipse.swt.browser.WebKit.convertToJS(WebKit.java:2411) returns something unexpected. I get to the conclusion that the general removal of SWT if the root cause is one of the above exception types, has too many exceptions to be reliable. What do you think?
(In reply to Marcel Bruch from comment #2) > Please review [1]. It's an IAE but it occurs deeply inside an SWT stack. > Until Display.readAndDispatch there is even no other project involved. To me > this seems like a problem in SWT rather than in any of SWT's clients. > I'll adjust the project guessing so that in those cases - if only SWT is on the > stack and it's a SWT client exception type - the problem will (still) be associated with SWT. > [1] > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 55d9bd89e4b0f0b83a6ec4dc/details Yes, this is can be assigned to SWT based on the above guessing logic as the client code here is SWT code. (In reply to Marcel Bruch from comment #3) > (In reply to Marcel Bruch from comment #2) > > I need your advice. > > There are corner cases I don't know how to handle properly yet. [1], for > instance, is an IAE which is a client's fault. But I've been teached that > every frame below Display.readAndDispatch *may or may not* be related to the > exception above and I should cut off the analysis after that frame. > > Is this general rule wrong? For [1] it looks like. But is there a general > pattern how I can reliably assign this problem to the right project? > > And: What is the right project? PartRenderingEngine$4.run is causing the > problem but likely on behalf of one of its clients which cannot be found on > the stack trace. > > It looks like we need a better logging in those cases. > > > > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 55d9bd89e4b0f0b83a6ec4dc/details IAE is thrown whenever one of the SWT API methods is invoked with an illegal argument. And in many places the APIs are called within the SWT code itself, in the SWT Custom classes like StyledText, CTabFolder, etc. In the above stack the client is SWT code itself and the SWT API is called as a result of user action (key down event). So, the right project to assign to is SWT. I think the general rule you mentioned is not wrong and you can cut off analysis below Display.readAndDispatch().
> I think the general rule you mentioned is not wrong and you can cut off > analysis below Display.readAndDispatch(). To explain further, Display.readAndDispatch() is the event loop which listens to any events occurring on the UI like key down or mouse down. Clients like Platform UI, JDT UI or even SWT have listeners attached to these events and when an event occurs the attached listener code is called and event handling happens. The listener code can call the SWT APIs for different actions. So, its safe to say that when there is Display.readAndDispatch() in the stack it's the UI thread and all the action happens above it and the stack below it can be ignored.
(In reply to Marcel Bruch from comment #1) > Can you please confirm that SWTError is actually a client exception? I > remember "No more handles" exceptions use SWTError and I don't think that > this is caused by a client/caller of SWT. But my understanding may be wrong. SWTError is thrown whenever a non-recoverable error happens internally in SWT. So, it's not exactly a client exception, but it can happen when something is wrong on the client side setup. For eg: https://dev.eclipse.org/recommenders/committers/confess/#/problems/55ca27efe4b08f80b00c30b1/details It can also happen when we run out of Operating System resources. (In reply to Marcel Bruch from comment #4) > "org.eclipse.swt.SWTException: Return value not valid" > > in > > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 559ba76ce4b079cbdd2cbd84/details > > > also does not look like a SWT client's fault. It looks like > org.eclipse.swt.browser.WebKit.convertToJS(WebKit.java:2411) returns > something unexpected. > > SWTException is thrown whenever a recoverable error happens in SWT. This happens most of the time when there is a problem with client code. For eg: invalid thread access or widget disposed. But, in your example the problem seems to be in SWT code itself. > I get to the conclusion that the general removal of SWT if the root cause is > one of the above exception types, has too many exceptions to be reliable. > > What do you think? I think we can use the same rule you mentioned - if only SWT is on the stack and it's a SWT client exception type (IAE, SWTException and SWTError) - the problem will (still) be associated with SWT.
Okay, those rules are in place. Let's see how they work out in the next weeks.
(In reply to Lakshmi Shanmugam from comment #5) > I think the general rule you mentioned is not wrong and you can cut off > analysis below Display.readAndDispatch(). When there are more than Display.readAndDispatch() in the stack, we should use the 1st call (the one lowest on the stack) as the cut off. For eg: https://dev.eclipse.org/recommenders/committers/confess/#/problems/55f23624e4b0d1d58e99ed7b/details https://dev.eclipse.org/recommenders/committers/confess/#/problems/55f2ecdbe4b0d1d58e9a05bb/details are problems in the client code, but are currently identified as SWT problems. Should I open a new bug for this, or can it be fixed as part of this bug?
(In reply to Lakshmi Shanmugam from comment #9) > (In reply to Lakshmi Shanmugam from comment #5) > > > I think the general rule you mentioned is not wrong and you can cut off > > analysis below Display.readAndDispatch(). > When there are more than Display.readAndDispatch() in the stack, we should > use the 1st call (the one lowest on the stack) as the cut off. > For eg: > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 55f23624e4b0d1d58e99ed7b/details > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 55f2ecdbe4b0d1d58e9a05bb/details > are problems in the client code, but are currently identified as SWT > problems. > Should I open a new bug for this, or can it be fixed as part of this bug? Another example: https://dev.eclipse.org/recommenders/committers/confess/#/problems/55ca196be4b0b79e0e811977/details
I'll create a list all problems that have more than one Display.readAndDispatch for review. If that behavior works well for all problems in there, I'd be happy. As long as we do not mix too many different topics into one bug, I don't need a new bug :-)
(In reply to Lakshmi Shanmugam from comment #7) > I think we can use the same rule you mentioned - if only SWT is on the stack > and it's a SWT client exception type (IAE, SWTException and SWTError) - the > problem will (still) be associated with SWT. Found a problem in one error report in the weekly digest. This is an SWTException caused by client code and doesn't match the above rule (there is another project in the stack) but still got assigned to SWT. https://dev.eclipse.org/recommenders/committers/confess/#/problems/55f19cafe4b0d1d58e99e124/details'
True. I also considered org.eclipse.swt.widgets.EventTable.sendEvent as a stop frame (similar to Display.readAndDispatch). I had the impression that this makes sense on at least some bugs. Will come up with a list of problems where EventTable.sendEvent is used as stop frame for re-evaluation.
Created attachment 256557 [details] Problems with more than one Display.readAndDispatch() I got a few problems where taking the lowest Display.readAndDispatch would lead to unintended results: https://dev.eclipse.org/recommenders/committers/confess/#/problems/54c4eeb1bee810030da05ee5 would add EPP logging although only the marketplace client is involved here. https://dev.eclipse.org/recommenders/committers/confess/#/problems/55cef26fe4b0f0b83a6dd600/ would add eclipse.ui (b/c of PropertyDialogAction) to the list of potential projects. I think the simple rule "Take the lowest Display.readAndDispatch" will not work out well enough. Is there anything in these traces that would allow me to decide where to cut off the trace?
(In reply to Marcel Bruch from comment #14) > Created attachment 256557 [details] > Problems with more than one Display.readAndDispatch() > > I got a few problems where taking the lowest Display.readAndDispatch would > lead to unintended results: > > > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 54c4eeb1bee810030da05ee5 would add EPP logging although only the marketplace > client is involved here. > Sorry, I'm not sure if I understand correctly how the tool assigns problems to projects. My understanding was that the project on top of the stack will be the one the problem will be assigned to. In this case, org.eclipse.epp.internal.mpc.ui.wizards.MarketplaceDropAdapter$WorkbenchListener.runUpdate(MarketplaceDropAdapter.java:365) is on the top, so problem should go to marketplace client. Is it right? In that case, how does cutting off only the trace below the lowest readAndDispatch, change the analysis? > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > 55cef26fe4b0f0b83a6dd600/ would add eclipse.ui (b/c of PropertyDialogAction) > to the list of potential projects. > In this case, there are 2 stack traces. Which one will be analyzed? I think the potential project would be jface in this case(b/c of org.eclipse.jface.wizard.ProgressMonitorPart.done(ProgressMonitorPart.java:179) in the 2nd stack).
(In reply to Lakshmi Shanmugam from comment #15) > (In reply to Marcel Bruch from comment #14) > > Created attachment 256557 [details] > > Problems with more than one Display.readAndDispatch() > > > > I got a few problems where taking the lowest Display.readAndDispatch would > > lead to unintended results: > > > > > > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > > 54c4eeb1bee810030da05ee5 would add EPP logging although only the marketplace > > client is involved here. > > > Sorry, I'm not sure if I understand correctly how the tool assigns problems > to projects. > My understanding was that the project on top of the stack will be the one > the problem will be assigned to. In this case, > org.eclipse.epp.internal.mpc.ui.wizards. > MarketplaceDropAdapter$WorkbenchListener.runUpdate(MarketplaceDropAdapter. > java:365) is on the top, so problem should go to marketplace client. > Is it right? In that case, how does cutting off only the trace below the > lowest readAndDispatch, change the analysis? Two general comments first 1. We do not use the topmost project in general. 2. A problem can be assigned to more than one project Often, and similar to SWT, the problem may be caused by the caller of a library function. Xtext is one such example. It is used by many projects but we have filters in place that remove Xtext when , e.g., OCL is on the stacktrace. How the project guessing works: Given an exception, we take exception and first guess a list of all affected projects. The list of affected projects is created by mapping each class on the stack trace to its declaring project (based on its package prefix). Then we apply several heuristics like "Is SWT project on top?" if not, remove SWT from the list. "Are Xtext AND OCL projects in the list?" If so, remove Xtext. etc. There are a few more heuristics which are more complicated than others but at the end they work similar: inspect the trace and decide - based on some creiteria - which projects to remove from the list of (potentially) affected projects. In the case of Display.readAndDispatch, we usually remove all projects that show up after the first occurrence of Display.readAndDispatch. I hope that explains why stoping after the the last Display.readAndDispatch would add EPP logging to EPP Marketplace. Having said this, I wonder whether we can define a better cut off rule by looking at which frames precede or follow Display.readAndDispatch: I noticed sequences like at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3983) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3660) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4172) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3761) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4147) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3764) at org.eclipse.jface.window.Window.runEventLoop(Window.java:827) at org.eclipse.jface.window.Window.open(Window.java:803) Question: Does Display.runAsyncMessages mean that the following frames are totally independent of the previous Display.readAndDispatch section? Would that make a better separator than taking the lowest Display.readAndDispatch? I don't know much about how SWT schedules async calls and the opening of windows. But maybe you know a whether there is a pattern? > > https://dev.eclipse.org/recommenders/committers/confess/#/problems/ > > 55cef26fe4b0f0b83a6dd600/ would add eclipse.ui (b/c of PropertyDialogAction) > > to the list of potential projects. > > > In this case, there are 2 stack traces. Which one will be analyzed? > I think the potential project would be jface in this case(b/c of > org.eclipse.jface.wizard.ProgressMonitorPart.done(ProgressMonitorPart.java: > 179) in the 2nd stack). We usually take the root cause of an exception. Actually we build a "virtual stacktrace" but these details don't really matter IMHO.
There are some changes in place. Closing this bug due to inactivity.
