Bug 285130 - reuse same background thread to process most target events
Prevents JDI events fork large number of background jobs, which
result in OutOfMemoryError: unable to create new native thread in
extreme cases.
Events are processed in the same order they were provided by the
target jvm. Events that require expression evaluation are then
forked to separate jobs (to avoid deadlock waiting on the main
event processing job).
Change-Id: Iddfff226dd101eb5288ae372b1511280e8fc7db8
Signed-off-by: Igor Fedorenko <igor@ifedorenko.com>
diff --git a/org.eclipse.jdt.debug.tests/testprograms/BulkThreadCreationTest.java b/org.eclipse.jdt.debug.tests/testprograms/BulkThreadCreationTest.java
new file mode 100644
index 0000000..ca0dad1
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/testprograms/BulkThreadCreationTest.java
@@ -0,0 +1,27 @@
+/*******************************************************************************
+ * Copyright (c) 2017 salesforce.com.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * salesforce.com - initial API and implementation
+ *******************************************************************************/
+
+public class BulkThreadCreationTest {
+
+ public static void main(String[] args) throws Exception {
+ int count = 1000;
+ Thread[] threads = new Thread[count];
+ for (int i = 0; i < count; i++) {
+ threads[i] = new Thread("bulk-" + i);
+ threads[i].start();
+ }
+ for (int i = 0; i < count; i++) {
+ threads[i].join();
+ }
+ return;
+ }
+
+}
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
index 9687b15..395fb8f 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
@@ -201,7 +201,7 @@
"bug329294", "bug401270", "org.eclipse.debug.tests.targets.HcrClass2", "org.eclipse.debug.tests.targets.HcrClass3", "org.eclipse.debug.tests.targets.HcrClass4",
"org.eclipse.debug.tests.targets.HcrClass5", "org.eclipse.debug.tests.targets.HcrClass6", "org.eclipse.debug.tests.targets.HcrClass7", "org.eclipse.debug.tests.targets.HcrClass8",
"org.eclipse.debug.tests.targets.HcrClass9", "TestContributedStepFilterClass", "TerminateAll_01", "TerminateAll_02", "StepResult1",
- "StepResult2", "StepResult3", "StepUncaught", "TriggerPoint_01" };
+ "StepResult2", "StepResult3", "StepUncaught", "TriggerPoint_01", "BulkThreadCreationTest" };
final String[] LAUNCH_CONFIG_NAMES_1_8 = {"LargeSourceFile"};
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java
index 6e8ee0f..7c772c8 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java
@@ -27,6 +27,7 @@
import org.eclipse.jdt.debug.tests.breakpoints.HitCountBreakpointsTests;
import org.eclipse.jdt.debug.tests.breakpoints.ImportBreakpointsTest;
import org.eclipse.jdt.debug.tests.breakpoints.JavaBreakpointListenerTests;
+import org.eclipse.jdt.debug.tests.breakpoints.JavaThreadEventHandlerTests;
import org.eclipse.jdt.debug.tests.breakpoints.MethodBreakpointTests;
import org.eclipse.jdt.debug.tests.breakpoints.MethodBreakpointTests15;
import org.eclipse.jdt.debug.tests.breakpoints.MiscBreakpointsTests;
@@ -38,7 +39,6 @@
import org.eclipse.jdt.debug.tests.breakpoints.TestToggleBreakpointsTarget;
import org.eclipse.jdt.debug.tests.breakpoints.TestToggleBreakpointsTarget8;
import org.eclipse.jdt.debug.tests.breakpoints.ThreadFilterBreakpointsTests;
-import org.eclipse.jdt.debug.tests.breakpoints.JavaThreadEventHandlerTests;
import org.eclipse.jdt.debug.tests.breakpoints.TriggerPointBreakpointsTests;
import org.eclipse.jdt.debug.tests.breakpoints.TypeNameBreakpointTests;
import org.eclipse.jdt.debug.tests.breakpoints.WatchpointTests;
@@ -58,6 +58,7 @@
import org.eclipse.jdt.debug.tests.core.DebugEventTests;
import org.eclipse.jdt.debug.tests.core.EEDefinitionTests;
import org.eclipse.jdt.debug.tests.core.EnvironmentTests;
+import org.eclipse.jdt.debug.tests.core.EventDispatcherTest;
import org.eclipse.jdt.debug.tests.core.EventSetTests;
import org.eclipse.jdt.debug.tests.core.ExecutionEnvironmentTests;
import org.eclipse.jdt.debug.tests.core.HcrTests;
@@ -277,6 +278,7 @@
addTest(new TestSuite(StratumTests.class));
addTest(new TestSuite(JavaDebugTargetTests.class));
addTest(new TestSuite(WorkingDirectoryTests.class));
+ addTest(new TestSuite(EventDispatcherTest.class));
// Refactoring tests
//TODO: project rename
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/core/EventDispatcherTest.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/core/EventDispatcherTest.java
new file mode 100644
index 0000000..18a5d45
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/core/EventDispatcherTest.java
@@ -0,0 +1,127 @@
+/*******************************************************************************
+ * Copyright (c) 2017 salesforce.com.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * salesforce.com - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.debug.tests.core;
+
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.eclipse.core.runtime.jobs.IJobChangeEvent;
+import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.core.runtime.jobs.JobChangeAdapter;
+import org.eclipse.debug.core.DebugEvent;
+import org.eclipse.debug.core.DebugException;
+import org.eclipse.debug.core.DebugPlugin;
+import org.eclipse.debug.core.IDebugEventSetListener;
+import org.eclipse.debug.core.model.IStackFrame;
+import org.eclipse.jdt.debug.core.IJavaLineBreakpoint;
+import org.eclipse.jdt.debug.core.IJavaThread;
+import org.eclipse.jdt.debug.tests.AbstractDebugTest;
+import org.eclipse.jdt.internal.debug.core.EventDispatcher.AbstractDispatchJob;
+import org.eclipse.jdt.internal.debug.core.model.JDIThread;
+
+public class EventDispatcherTest extends AbstractDebugTest {
+
+ private JobChangeAdapter jobListener;
+ private Map<AbstractDispatchJob, Object> jobs;
+
+ public EventDispatcherTest(String name) {
+ super(name);
+ }
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ jobs = Collections.synchronizedMap(new IdentityHashMap<AbstractDispatchJob, Object>());
+ jobListener = new JobChangeAdapter() {
+ @Override
+ public void aboutToRun(IJobChangeEvent event) {
+ Job job = event.getJob();
+ if (job instanceof AbstractDispatchJob) {
+ jobs.put((AbstractDispatchJob) job, Boolean.TRUE);
+ }
+ }
+ };
+ Job.getJobManager().addJobChangeListener(jobListener);
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ if (jobListener != null) {
+ Job.getJobManager().removeJobChangeListener(jobListener);
+ }
+ super.tearDown();
+ }
+
+ public void test285130_bulkThreadCreation() throws Exception {
+ // the real goal of this test is to validate that rapidly delivered JDI events do not start large number of threads
+ // unfortunately there is no direct way to observe startup of the JDI event processing threads
+ // as approximation, assert single JDI processing Job was ever run during the test
+ // as sanity check, assert expected number of DebugEvent was delivered during the test
+
+ // sanity check: count number of JDIThread thread-create events
+ AtomicInteger threadCount = new AtomicInteger();
+ IDebugEventSetListener debugListener = events -> {
+ for (DebugEvent event : events) {
+ if (event.getKind() == DebugEvent.CREATE && event.getSource() instanceof JDIThread) {
+ JDIThread thread = (JDIThread) event.getSource();
+ try {
+ if (thread.getName().startsWith("bulk-")) {
+ threadCount.incrementAndGet();
+ }
+ }
+ catch (DebugException e) {
+ }
+ }
+ }
+ };
+ try {
+ DebugPlugin.getDefault().addDebugEventListener(debugListener);
+ createLineBreakpoint(24, "BulkThreadCreationTest");
+ launchToBreakpoint("BulkThreadCreationTest");
+ }
+ finally {
+ removeAllBreakpoints();
+ DebugPlugin.getDefault().removeDebugEventListener(debugListener);
+ }
+
+ assertEquals("Unexpected number of JDIThread thread-create events", 1000, threadCount.get());
+ assertEquals("Unexpected number of event dispatching jobs: " + jobs.size() + " | " + jobs.keySet(), 0, jobs.size());
+ }
+
+ /**
+ * Tests that a conditional breakpoint with an expression that will hit a breakpoint will complete the conditional expression evaluation (bug
+ * 269231) and that we dispatch events for conditional breakpoints in dedicated jobs.
+ *
+ * @throws Exception
+ */
+ public void testConditionalExpressionEventDispatching() throws Exception {
+ String typeName = "BreakpointListenerTest";
+ createConditionalLineBreakpoint(15, typeName, "foo(); return false;", true);
+ IJavaLineBreakpoint breakpoint = createLineBreakpoint(20, typeName);
+ IJavaThread thread = null;
+ try {
+ thread = launchToLineBreakpoint(typeName, breakpoint);
+ IStackFrame top = thread.getTopStackFrame();
+ assertNotNull("Missing top frame", top);
+ assertTrue("Thread should be suspended", thread.isSuspended());
+ assertEquals("Wrong location", breakpoint.getLineNumber(), top.getLineNumber());
+ }
+ finally {
+ terminateAndRemove(thread);
+ removeAllBreakpoints();
+ }
+ // Expect to see two jobs for conditional breakpoint with 1) class prepare and 2) breakpoint hit events
+ assertEquals("Unexpected number of event dispatching jobs: " + jobs.size() + " | " + jobs.keySet(), 2, jobs.size());
+ }
+
+}
\ No newline at end of file
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/EventDispatcher.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/EventDispatcher.java
index 216b9c4..299d99b 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/EventDispatcher.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/EventDispatcher.java
@@ -65,7 +65,7 @@
/**
* Whether this dispatcher is shutdown.
*/
- private boolean fShutdown;
+ private volatile boolean fShutdown;
/**
* Table of event listeners. Table is a mapping of <code>EventRequest</code>
* to <code>IJDIEventListener</code>.
@@ -220,6 +220,50 @@
}
}
+ private boolean requiresExpressionEvaluation(EventSet eventSet) {
+ EventIterator iter = eventSet.eventIterator();
+ while (iter.hasNext()) {
+ Event event = iter.nextEvent();
+ if (event == null) {
+ continue;
+ }
+ IJDIEventListener listener = fEventHandlers.get(event.request());
+ if (listener instanceof IJavaLineBreakpoint) {
+ try {
+ if (((IJavaLineBreakpoint) listener).isConditionEnabled()) {
+ return true;
+ }
+ }
+ catch (CoreException e) {
+ return true; // assume the worst
+ }
+ }
+ }
+ return false;
+ }
+
+ /** @noreference public for test purposes */
+ public abstract class AbstractDispatchJob extends Job {
+ protected AbstractDispatchJob(String name) {
+ super(name);
+ }
+
+ @Override
+ public boolean belongsTo(Object family) {
+ return family == EventDispatcher.class || family == EventDispatcher.this;
+ }
+
+ @Override
+ public String toString() {
+ try {
+ return super.toString() + " for [" + fTarget.getName() + "]"; //$NON-NLS-1$ //$NON-NLS-2$
+ }
+ catch (DebugException e) {
+ return super.toString();
+ }
+ }
+ }
+
/**
* Continuously reads events that are coming from the event queue, until
* this event dispatcher is shutdown. A debug target starts a thread on this
@@ -232,9 +276,9 @@
VirtualMachine vm = fTarget.getVM();
if (vm != null) {
EventQueue q = vm.eventQueue();
- EventSet eventSet = null;
while (!isShutdown()) {
try {
+ EventSet eventSet;
try {
// Get the next event set.
eventSet = q.remove(1000);
@@ -242,42 +286,21 @@
break;
}
- if (!isShutdown() && eventSet != null) {
- final EventSet set = eventSet;
- Job job = new Job("JDI Event Dispatch") { //$NON-NLS-1$
- @Override
- protected IStatus run(IProgressMonitor monitor) {
- dispatch(set);
- return Status.OK_STATUS;
- }
-
- @Override
- public boolean belongsTo(Object family) {
- if (family instanceof Class) {
- Class<?> clazz = (Class<?>) family;
- EventIterator iterator = set.eventIterator();
- while (iterator.hasNext()) {
- Event event = iterator.nextEvent();
- if (clazz.isInstance(event)) {
- return true;
- }
- }
+ if (eventSet != null) {
+ if (!requiresExpressionEvaluation(eventSet)) {
+ dispatch(eventSet);
+ } else {
+ // 269231 always evaluate expressions in a separate job to avoid deadlocks
+ Job job = new AbstractDispatchJob("JDI Expression Evaluation Event Dispatch") { //$NON-NLS-1$
+ @Override
+ protected IStatus run(IProgressMonitor monitor) {
+ dispatch(eventSet);
+ return Status.OK_STATUS;
}
- return false;
- }
-
- @Override
- public String toString() {
- try {
- return super.toString() + " for [" + fTarget.getName() + "]"; //$NON-NLS-1$ //$NON-NLS-2$
- }
- catch (DebugException e) {
- return super.toString();
- }
- }
- };
- job.setSystem(true);
- job.schedule();
+ };
+ job.setSystem(true);
+ job.schedule();
+ }
}
} catch (InterruptedException e) {
break;
@@ -293,6 +316,7 @@
*/
public void shutdown() {
fShutdown = true;
+ Job.getJobManager().cancel(this);
}
/**
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java
index cad7552..ba6a3fe 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java
@@ -22,9 +22,7 @@
import org.eclipse.core.resources.IMarker;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.MultiStatus;
-import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.core.runtime.Platform;
-import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.debug.core.DebugEvent;
import org.eclipse.debug.core.DebugPlugin;
import org.eclipse.debug.core.IDebugEventSetListener;
@@ -53,7 +51,6 @@
import com.sun.jdi.event.Event;
import com.sun.jdi.event.EventSet;
import com.sun.jdi.event.LocatableEvent;
-import com.sun.jdi.event.ThreadStartEvent;
import com.sun.jdi.request.ClassPrepareRequest;
import com.sun.jdi.request.EventRequest;
import com.sun.jdi.request.EventRequestManager;
@@ -307,13 +304,6 @@
ThreadReference threadRef = ((LocatableEvent) event).thread();
JDIThread thread = target.findThread(threadRef);
if (thread == null) {
- // wait for any thread start event sets to complete processing
- // see bug 271700
- try {
- Job.getJobManager().join(ThreadStartEvent.class, null);
- } catch (OperationCanceledException e) {
- } catch (InterruptedException e) {
- }
thread = target.findThread(threadRef);
}
if (thread == null || thread.isIgnoringBreakpoints()) {
@@ -425,8 +415,9 @@
protected boolean installableReferenceType(ReferenceType type,
JDIDebugTarget target) throws CoreException {
String installableType = getTypeName();
- if (installableType == null )
+ if (installableType == null ) {
return false;
+ }
String queriedType = type.name();
if( queriedType == null) {
return false;
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java
index 01ae84a..320cefa 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java
@@ -2440,13 +2440,6 @@
ThreadReference ref = ((ThreadDeathEvent) event).thread();
JDIThread thread = findThread(ref);
if (thread == null) {
- // wait for any thread start event sets to complete processing
- // see bug 272494
- try {
- Job.getJobManager().join(ThreadStartEvent.class, null);
- } catch (OperationCanceledException e) {
- } catch (InterruptedException e) {
- }
thread = target.findThread(ref);
}
if (thread != null) {
