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

Bug 335251

Summary:

InterruptedException being logged by WorkbenchResourceHelper

Product:

[WebTools] WTP Common Tools

Reporter:

Aidyl Kareh <amkareh>

Component:

wst.common

Assignee:

Roberto Sanchez Herrera <shr31223>

Status:

NEW ---

QA Contact:

Carl Anderson <ccc>

Severity:

normal

Priority:

CC:

amkareh, david_williams

Version:

unspecified

Target Milestone:

Future

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Proposed Patch	none

Description Aidyl Kareh

2011-01-24 16:33:56 EST

Build Identifier: WTP 3.2.3

I saw the following InterruptedException in my logs:

java.lang.InterruptedException
	at org.eclipse.core.internal.jobs.OrderedLock.acquire(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.WorkbenchResourceHelper$FileAdapter.aquireSaveLock(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.WorkbenchResourceHelper$FileAdapter.isConsistent(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.WorkbenchResourceHelper.isConsistent(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.integration.ResourceSetWorkbenchEditSynchronizer.addedResource(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.integration.ResourceSetWorkbenchEditSynchronizer.visit(Unknown Source)
	at org.eclipse.core.internal.events.ResourceDelta.accept(Unknown Source)
	at org.eclipse.core.internal.events.ResourceDelta.accept(Unknown Source)
	at org.eclipse.core.internal.events.ResourceDelta.accept(Unknown Source)
	at org.eclipse.core.internal.events.ResourceDelta.accept(Unknown Source)
	at org.eclipse.core.internal.events.ResourceDelta.accept(Unknown Source)
	at org.eclipse.core.internal.events.ResourceDelta.accept(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.integration.ResourceSetWorkbenchEditSynchronizer.primAcceptDelta(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.integration.ResourceSetWorkbenchEditSynchronizer.acceptDelta(Unknown Source)
	at org.eclipse.wst.common.internal.emfworkbench.integration.ResourceSetWorkbenchEditSynchronizer.resourceChanged(Unknown Source)
	at org.eclipse.core.internal.events.NotificationManager$2.run(Unknown Source)
	at org.eclipse.core.runtime.SafeRunner.run(Unknown Source)
	at org.eclipse.core.internal.events.NotificationManager.notify(Unknown Source)
	at org.eclipse.core.internal.events.NotificationManager.broadcastChanges(Unknown Source)
	at org.eclipse.core.internal.resources.Workspace.broadcastPostChange(Unknown Source)
	at org.eclipse.core.internal.resources.Workspace.endOperation(Unknown Source)
	at org.eclipse.core.internal.resources.Workspace.run(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaModelManager.initializeAllContainers(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaModelManager.getClasspathContainer(Unknown Source)
	at org.eclipse.jdt.core.JavaCore.getClasspathContainer(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaProject.resolveClasspath(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaProject.resolveClasspath(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaProject.getResolvedClasspath(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaProject.buildStructure(Unknown Source)
	at org.eclipse.jdt.internal.core.Openable.generateInfos(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaElement.openWhenClosed(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaElement.getChildren(Unknown Source)
	at org.eclipse.jdt.internal.core.JavaProject.getPackageFragmentRoots(Unknown Source)


Reproducible: Always

Comment 1 Aidyl Kareh

2011-01-24 16:49:31 EST

Created attachment 187479 [details]
Proposed Patch

Currently we are just logging the exception and nothing else is done. The
proposed patch would catch the InterruptedException and preserve evidence that the interruption occurred so that code higher up on the call stack can learn of the interruption and respond to it if it wants to.

Comment 2 Aidyl Kareh

2011-01-24 17:13:47 EST

    * Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 

An adopter product has been showing this exception in its logs. This exception is currently just being logged. The correct behavior would be to catch the InterruptedException and preserve evidence that the interruption occurred so that code higher up on the call stack can learn of the interruption and respond to it if it wants to.


    * Is there a work-around? If so, why do you believe the work-around is insufficient? 

No.


    * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added?

Was unable to reproduce exception on demand. Code review only.


    * Give a brief technical overview. Who has reviewed this fix? 

This patch catches the InterruptedException received while trying to acquire a lock and changes the thread's interrupt status so that upstream code can react to the interruption if necessary. Carl Anderson and Jason Sholl reviewed this patch.


    * What is the risk associated with this fix? 

This patch has minimal risk since the function's behavior is not changing and the exception is not being thrown up the stack. Only the thread's interrupt status is changed so code that wants to react, can react.

Comment 3 David Williams

2011-01-25 10:45:10 EST

Just from a casual glance, I'm not sure why you need to catch the interrupt at all. Plus, If the interrupt occurs, during the 'aquireSaveLock', then it is not obvious that you want to still do the work that follows it (between the aquireSaveLock and the eventual 'return'. (You know, do you need the lock, or not?). 

Plus, I've a minor concern you are now "changing behavior" in a subtle way. Callers may get interrupted now, where they didn't before. Right? That my be fine ... I'm just wondering what the risk of that is? how that's normally documented in javadocs? 

I'm not exactly opposed to this fix, but would appreciate a second or re-review with my questions in mind, and if you are still convinced this is the right fix, then I could be convinced too. 

Thanks,

Comment 4 Carl Anderson

2011-01-27 11:30:15 EST

Retargeting this to WTP 3.2.4 - I had thought that we used Thread.currentThread().interrupted() to solve bug 313623 , but we eventually went in a different direction.  Let's revisit this solution.