Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GR-60633] Fix JDWP reconnect logic not resetting all state properly. #10420

Merged
merged 1 commit into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,6 @@ public interface FieldRef {
* @return true if this field has any breakpoints, false otherwise
*/
boolean hasActiveBreakpoint();

void disposeFieldBreakpoint();
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.oracle.truffle.api.frame.Frame;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.nodes.RootNode;
import com.oracle.truffle.espresso.jdwp.impl.DebuggerController;

/**
* Interface that defines required methods for a guest language when implementing JDWP.
Expand Down Expand Up @@ -500,4 +501,6 @@ public interface JDWPContext {
Object allocateInstance(KlassRef klass);

void steppingInProgress(Thread t, boolean value);

void replaceController(DebuggerController newController);
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ public interface MethodRef {
*/
boolean hasActiveHook();

void disposeHooks();

/**
* Determine if this method is obsolete. A method is obsolete if it has been replaced by a
* non-equivalent method using the RedefineClasses command. The original and redefined methods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ public void activate(Object mainThread, DebuggerController control, JDWPContext
this.ids = context.getIds();
}

public void replaceController(DebuggerController newController) {
this.debuggerController = newController;
}

@Override
public void onDetach() {
// free up request, to avoid attempting to send anything further
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,8 @@ public interface BreakpointInfo {
void addSuspendPolicy(byte suspendPolicy);

byte getSuspendPolicy();

default void dispose() {
// do nothing by default
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ public void reInitialize() {
DebuggerController newController = new DebuggerController(jdwpLogger);
newController.truffleContext = truffleContext;
newController.initialize(debugger, options, context, initialThread, eventListener);
context.replaceController(newController);
assert newController.setupState != null;

if (newController.setupState.fatalConnectionError) {
Expand Down Expand Up @@ -186,6 +187,10 @@ public void reset(boolean prepareForReconnect) {
// existence state.
resetting.lockInterruptibly();

// end the current debugger session to avoid hitting any further breakpoints
// when resuming all threads
endSession();

currentReceiverThread = receiverThread;

// Close the server socket used to listen for transport dt_socket.
Expand All @@ -207,16 +212,14 @@ public void reset(boolean prepareForReconnect) {
// re-enable GC for all objects
getGCPrevention().clearAll();

// end the current debugger session to avoid hitting any further breakpoints
// when resuming all threads
endSession();

// resume all threads
forceResumeAll();
eventFilters.clearAll();

// Now, close the socket, which will force the receiver thread to complete eventually.
// Note that we might run this code in the receiver thread, so we can't simply join.
closeSocket();

// resume all threads
forceResumeAll();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} finally {
Expand Down Expand Up @@ -822,8 +825,8 @@ private void lockThread(Object thread, boolean forceSuspend, List<Callable<Void>
while (!Thread.currentThread().isInterrupted()) {
try {
synchronized (lock) {
if (!lock.isLocked()) {
// released from other thread, so break loop
if (!lock.isLocked() || !connection.isOpen()) {
// released from other thread or session ended, so break loop
break;
}
// no reason to hold a hard suspension status, since now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,20 @@ public RequestFilter getRequestFilter(int requestId) {
readLock.unlock();
}
}

public void clearAll() {
try {
lock.writeLock().lock();
// traverse all filters and clear all registered breakpoint information
for (RequestFilter requestFilter : requestFilters) {
BreakpointInfo breakpointInfo = requestFilter.getBreakpointInfo();
if (breakpointInfo != null) {
breakpointInfo.dispose();
}
}
requestFilters = new RequestFilter[0];
} finally {
lock.writeLock().unlock();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,9 @@ public boolean isModificationBreakpoint() {
public boolean isAccessBreakpoint() {
return accessBreakpoint;
}

@Override
public void dispose() {
field.disposeFieldBreakpoint();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,11 @@ public boolean onMethodEnter(@SuppressWarnings("unused") MethodRef method, @Supp
public boolean onMethodExit(@SuppressWarnings("unused") MethodRef method, @SuppressWarnings("unused") Object returnValue) {
return isMethodExit;
}

@Override
public void dispose() {
for (MethodRef method : methods) {
method.disposeHooks();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -926,11 +926,16 @@ public final void removeFieldBreakpointInfo(int requestId) {
// remove index 1, but keep info at index 0
temp[0] = infos[0];
infos = temp;
return;
}
}
}

@Override
public void disposeFieldBreakpoint() {
hasActiveBreakpoints.set(false);
infos = null;
}

public void setCompatibleField(@SuppressWarnings("unused") Field field) {
// only applicable to RedefineAddedFields
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,11 @@ public synchronized void removeActiveHook(MethodHook hook) {
}
}

public synchronized void disposeHooks() {
hasActiveHook.set(false);
hooks = MethodHook.EMPTY;
}

public SharedRedefinitionContent redefine(ObjectKlass.KlassVersion klassVersion, ParserMethod newMethod, ParserKlass newKlass, Ids<Object> ids) {
// install the new method version immediately
LinkedMethod newLinkedMethod = new LinkedMethod(newMethod);
Expand Down Expand Up @@ -1848,6 +1853,11 @@ public void removedMethodHook(MethodHook hook) {
getMethod().removeActiveHook(hook);
}

@Override
public void disposeHooks() {
getMethod().disposeHooks();
}

@Override
public boolean hasActiveHook() {
return getMethod().hasActiveHook();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,20 @@ public final class JDWPContextImpl implements JDWPContext {
private RedefinitionPluginHandler redefinitionPluginHandler;
private final ArrayList<ReloadingAction> classInitializerActions = new ArrayList<>(1);
private DebuggerController controller;
private VMEventListenerImpl vmEventListener;

public JDWPContextImpl(EspressoContext context) {
this.context = context;
this.ids = new Ids<>(StaticObject.NULL);
this.innerClassRedefiner = new InnerClassRedefiner(context);
}

public void jdwpInit(TruffleLanguage.Env env, Object mainThread, VMEventListenerImpl vmEventListener) {
public void jdwpInit(TruffleLanguage.Env env, Object mainThread, VMEventListenerImpl eventListener) {
Debugger debugger = env.lookup(env.getInstruments().get("debugger"), Debugger.class);
this.controller = env.lookup(env.getInstruments().get(JDWPInstrument.ID), DebuggerController.class);
vmEventListener.activate(mainThread, controller, this);
controller.initialize(debugger, context.getEspressoEnv().JDWPOptions, this, mainThread, vmEventListener);
vmEventListener = eventListener;
eventListener.activate(mainThread, controller, this);
controller.initialize(debugger, context.getEspressoEnv().JDWPOptions, this, mainThread, eventListener);
redefinitionPluginHandler = RedefinitionPluginHandler.create(context);
classRedefinition = context.createClassRedefinition(ids, redefinitionPluginHandler);
}
Expand All @@ -120,6 +122,12 @@ public void finalizeContext() {
}
}

@Override
public void replaceController(DebuggerController newController) {
this.controller = newController;
vmEventListener.replaceController(newController);
}

@Override
public Ids<Object> getIds() {
return ids;
Expand Down