Description Simeon Andreev 2018-08-13 09:35:26 EDT

Created attachment 275380 [details]
Video showing how the race condition can be reproduced during a test.

The race condition which causes the bug is unfortunately complicated, see attached video to better understand how to reproduce. Reproduction steps are:

Our product does some JDI evaluations, whenever a suspended thread is selected in the Debug View. This sometimes leads to wrong selection in the Debug View. See attached JUnit test for example code.


1. Set breakponts at the following locations (see below for details, since line numbers change):

JDIThread [line: 817] - runEvaluation(IEvaluationRunnable, IProgressMonitor, int, boolean)	
JDIThread [line: 1024] - invokeMethod(ClassType, ObjectReference, Method, List<? extends Value>, boolean)	
JDIThread [line: 1054] - invokeMethod(ClassType, ObjectReference, Method, List<? extends Value>, boolean)	
JDIThread [line: 1493] - handleSuspendForBreakpoint(JavaBreakpoint, boolean)	
ThreadEventHandler [line: 255] - fireDeltaUpdatingSelectedFrame(IThread, int, DebugEvent)	
ThreadEventHandler [line: 265] - fireDeltaUpdatingSelectedFrame(IThread, int, DebugEvent)	

2. Debug attached test "BugXXXXXXTests.java" as Eclipse JUnit plug-in test.

3. Two breakpoints are simultaneously hit, JDIThread.handleSuspend and JDIThread.runEvaluation. The thread which hit the breakpoint in the test snippet (in the debugee Eclipse) is selected and suspended.
4. Resume breakpoint in JDIThread.handleSuspend.
5. Resume breakpoint in JDIThread.runEvaluation.
6. Two breakpoints are simultaneously hit, JDIThread.invokeMethod and ThreadEventHandler.fireDeltaUpdatingSelectedFrame. The one in JDIThread wants to set the state of the thread as "running". The one one in ThreadEventHandler wants to query the top stack frame.
7. Resume first breakpoint in JDIThread.invokeMethod. The state of the suspended thread is now set to "running".
8. Second breakpoint in JDIThread.invokeMethod is hit, at the call to invokeComplete. The thread state is still running.
9. Resume first breakpoint in ThreadEventHandler. This doesn't compute any stack frames due to the running thread state. Resulting delta therefore selects thread.
10. Second breakpoint in ThreadEventHandler.fireDeltaUpdatingSelectedFrame is hit, at the check of having no stack frame and a suspended thread.
11. Resume second breakpoint in JDIThread. The state of the thread is now "suspended" again.
12. Resume second breakpoint in ThreadEventHandler. Due to the now suspended state, this results in delta:

Model Delta Start
  Element: org.eclipse.debug.internal.core.LaunchManager@23aaa756
      Flags: NO_CHANGE
      Index: -1 Child Count: -1
    Element: org.eclipse.debug.core.Launch@3442f8f4
        Flags: NO_CHANGE
        Index: 0 Child Count: 2
      Element: org.eclipse.jdt.internal.debug.core.model.JDIDebugTarget@8eb9935
          Flags: NO_CHANGE
          Index: 0 Child Count: 5
        Element: org.eclipse.jdt.internal.debug.core.model.JDIThread@3b6037da
            Flags: EXPAND | SELECT | STATE | REVEAL | 
            Index: 4 Child Count: 3
Model Delta End

This delta results in selecting and expanding the thread, as opposed to its top stack frame. As a result, e.g. its not possible to use resume and stepping. Client plug-in code which depends on correct selection is also broken.


There are two race conditions here. The first occurs from step 1. to step 5.. The latter occurs from step 6. onward, and can only occur after the first race condition did.

For the first race condition, the following occurs:

    a) Debug target proxy installation is triggered "late" and detects a suspended thread.
    b) A selection is fired. Evaluation from example plug-in kicks in due to this.
    c) JDIThread.handleSuspendForBreakpoint(JavaBreakpoint, boolean) is not yet done with resetting the fSuspendVoteInProgress flag to false.
    d) The evaluation from the example plug-in sees fSuspendVoteInProgress==true and fires "quiet" deltas.
    e) The quiet deltas lead to collapsing the expanded and selected thread from step b), setting up for the second race condition.

In the second race condition, ThreadEventHandler.fireDeltaUpdatingSelectedFrame will create a bad delta due to the following:

    i) A JDI evaluation is running in paralle to fireDeltaUpdatingSelectedFrame. The evaluation sets the thread state to "running".
   ii) fireDeltaUpdatingSelectedFrame sees this and decides that there is no top stack frame in the thread.
  iii) The JDI evaluation finishes, setting the thread state to "suspended" again.
   iv) fireDeltaUpdatingSelectedFrame continues, sees that there is no top stack frame but that the thread is suspended. It creates the bad delta from above, breaking any code which depends on correct stack frame selection.


Here are the breakpoint locations (see also attached video), that can force the race conditions to occur:


Two breakpoints in JDIThread.involeMethod, to control the running state of the suspended thread for the second race condition:

{code}
	protected Value invokeMethod(ClassType receiverClass,
			ObjectReference receiverObject, Method method, List<? extends Value> args,
			boolean invokeNonvirtual) throws DebugException {
		...
			synchronized (this) {
				...
				setRequestTimeout(Integer.MAX_VALUE);
				setRunning(true);  // <------------ BREAKPOINT HERE
				setInvokingMethod(true);
			}
                ...
		invokeComplete(timeout);  // <---------- BREAKPOINT HERE
		return result;
	}
{code}

One breakpoint in JDIThread.handleSuspendForBreakpoint, to control the first race condition (due to fSuspendVoteInProgress):

{code}
	public boolean handleSuspendForBreakpoint(JavaBreakpoint breakpoint,
			boolean suspendVote) {
		...
		} finally {
			synchronized (this) {         // <------------- BREAKPOINT HERE
				fSuspendVoteInProgress = false;
				...
			}
		}
		return suspend;
	}
{code}

One breakpoint in JDIThread.runEvaluation:

        @Override
	public void runEvaluation(IEvaluationRunnable evaluation,
			IProgressMonitor monitor, int evaluationDetail,
			boolean hitBreakpoints) throws DebugException {
		...
		synchronized (fEvaluationLock) {
			fEvaluationRunnable = evaluation;
			fHonorBreakpoints = hitBreakpoints;
		}
		boolean quiet = isSuspendVoteInProgress();     // <--------------- BREAKPOINT HERE
		if (quiet) {
			// evaluations are quiet when a suspend vote is in progress
			// (conditional breakpoints, etc.).
			fireEvent(new DebugEvent(this, DebugEvent.MODEL_SPECIFIC,
					RESUME_QUIET));
		} ...
                ...
	}


Two breakpoints breakpoints in ThreadEventHandler.fireDeltaUpdatingSelectedFrame, again to control the second race condition:

	private void fireDeltaUpdatingSelectedFrame(IThread thread, int flags, DebugEvent event) {
		...
		try {
			Object frameToSelect = event.getData();
			if (frameToSelect == null || !(frameToSelect instanceof IStackFrame)) {
				frame = thread.getTopStackFrame();       // <------------------------------ BREAKPOINT HERE
			}
                        ...
    	        if (isEqual(frame, prev)) {
    		    if (frame == null) {
    			if (thread.isSuspended()) {                      // <------------------------------ BREAKPOINT HERE
	    			// no frames, but suspended - update & select
	    			node = node.addNode(thread, threadIndex, flags | IModelDelta.STATE | IModelDelta.SELECT, childCount);
    			}
    		        ...
    	        fireDelta(delta);
	}