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

Bug 332138

Summary: Stack overflow in P2TargetUtils
Product: [Eclipse Project] PDE Reporter: Andrew Niefer <aniefer>
Component: UIAssignee: Curtis Windatt <curtis.windatt.public>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: ankur_sharma, curtis.windatt.public, jeffmcaffer
Version: 3.7   
Target Milestone: 3.7 M4   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Proposed fix none

Description Andrew Niefer CLA 2010-12-08 10:23:03 EST
I was running  I20101206-1300 + pde.core from HEAD and had a self hosted workspace crash with a stack overflow:

at org.eclipse.pde.internal.core.target.P2TargetUtils.getGarbageCollector(P2TargetUtils.java:387)
at org.eclipse.pde.internal.core.target.P2TargetUtils.getAgent(P2TargetUtils.java:298)
at org.eclipse.pde.internal.core.target.P2TargetUtils.getGarbageCollector(P2TargetUtils.java:387)
at org.eclipse.pde.internal.core.target.P2TargetUtils.getAgent(P2TargetUtils.java:298)
...
Comment 1 Curtis Windatt CLA 2010-12-08 10:26:19 EST
Jeff, can you look at this.  We either need to prevent concurrent access to the method (or at least the agent creation section) or alter how we call the GC.

Consider for M4.
Comment 2 Curtis Windatt CLA 2010-12-08 12:01:10 EST
Created attachment 184799 [details]
Proposed fix

This fix avoids the stack overflow by getting the p2 garbage collector directly from the created agent rather than calling the utility method.
Comment 3 Curtis Windatt CLA 2010-12-08 12:03:22 EST
I have applied the patch to HEAD and will release for the next build towards M4.  Jeff, please take a look to see if the fix is reasonable or whether we need to do more in M5.
Comment 4 Jeff McAffer CLA 2010-12-08 13:06:17 EST
I took a quick look at the patch and it should do fine.  Thanks.  I'll take a more comprehensive look later and see if there are any refinements for M5.
Comment 5 Curtis Windatt CLA 2010-12-09 11:22:30 EST
Verified the fix is in I20101208-1300
Comment 6 Jeff McAffer CLA 2010-12-10 09:34:45 EST
I have to say I'm a little confused by how this stack overflow could be happening.  createAgent() creates the agent and registers it as a service.  getAgent looks for the registered service.  The only way this should loop is if somehow the service is not getting registered or not found.  

createAgent() does not have a failure path (other than runtime exceptions).

getAgent() can fail lookup if a) the service really is not there or b) if the location does not parse well in an LDAP filter.  a) should not happen since the values for the filter are the same on creation and lookup.  b) could be happening if the value of
	AGENT_LOCATION = URIUtil.fromString("file:" + PDECore.getDefault().getStateLocation().append(".p2"));
is something that causes an error when embedded in an LDAP filter.

My concern here is that with the fix we will prevent the stack overflow but the net result is that we'll be creating a new agent every time we look for one in whatever scenario Andrew has.

Andrew, any thoughts on how to reproduce this?  I've been running/testing selfhosted all along and its been working fine.  Is there something you can think of that would be unique about your situation?