From a9096e1f1923d04b60216a21408ddfbb411db78f Mon Sep 17 00:00:00 2001 From: Jinbo Wang Date: Mon, 22 Jun 2020 15:14:36 +0800 Subject: [PATCH 1/2] Improve the HCR workflow Signed-off-by: Jinbo Wang --- .../java/debug/core/adapter/ErrorCode.java | 3 +- .../handler/HotCodeReplaceHandler.java | 5 +++- .../java/debug/core/protocol/Responses.java | 9 ++++++ .../internal/JavaHotCodeReplaceProvider.java | 29 ++++++++++++++----- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ErrorCode.java b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ErrorCode.java index f39796909..1cdc4f9ce 100644 --- a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ErrorCode.java +++ b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ErrorCode.java @@ -34,7 +34,8 @@ public enum ErrorCode { COMPLETIONS_FAILURE(1017), EXCEPTION_INFO_FAILURE(1018), EVALUATION_COMPILE_ERROR(2001), - EVALUATE_NOT_SUSPENDED_THREAD(2002); + EVALUATE_NOT_SUSPENDED_THREAD(2002), + HCR_FAILURE(3001); private int id; diff --git a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java index f32d4503e..aae11a3a6 100644 --- a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java +++ b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java @@ -59,8 +59,11 @@ public CompletableFuture handle(Command command, Arguments arguments, IHotCodeReplaceProvider provider = context.getProvider(IHotCodeReplaceProvider.class); - return provider.redefineClasses().thenApply(classNames -> { + return provider.redefineClasses().thenCompose(classNames -> { response.body = new Responses.RedefineClassesResponse(classNames.toArray(new String[0])); + return CompletableFuture.completedFuture(response); + }).exceptionally(ex -> { + response.body = new Responses.RedefineClassesResponse(new String[0], ex.getMessage()); return response; }); } diff --git a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/protocol/Responses.java b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/protocol/Responses.java index b1c8d5ae3..1b4bc7b4a 100644 --- a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/protocol/Responses.java +++ b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/protocol/Responses.java @@ -299,12 +299,21 @@ public ExceptionInfoResponse(String exceptionId, String description, ExceptionBr public static class RedefineClassesResponse extends ResponseBody { public String[] changedClasses = new String[0]; + public String errorMessage = null; /** * Constructor. */ public RedefineClassesResponse(String[] changedClasses) { + this(changedClasses, null); + } + + /** + * Constructor. + */ + public RedefineClassesResponse(String[] changedClasses, String errorMessage) { this.changedClasses = changedClasses; + this.errorMessage = errorMessage; } } } diff --git a/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java b/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java index 3ea1635d2..cb7d6200f 100644 --- a/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java +++ b/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java @@ -53,6 +53,7 @@ import org.eclipse.jdt.core.util.IClassFileReader; import org.eclipse.jdt.core.util.ISourceAttribute; import org.eclipse.jdt.internal.core.util.Util; +import org.eclipse.jdt.ls.core.internal.JobHelpers; import com.microsoft.java.debug.core.Configuration; import com.microsoft.java.debug.core.DebugException; @@ -60,6 +61,8 @@ import com.microsoft.java.debug.core.DebugUtility; import com.microsoft.java.debug.core.IDebugSession; import com.microsoft.java.debug.core.StackFrameUtility; +import com.microsoft.java.debug.core.adapter.AdapterUtils; +import com.microsoft.java.debug.core.adapter.ErrorCode; import com.microsoft.java.debug.core.adapter.HotCodeReplaceEvent; import com.microsoft.java.debug.core.adapter.IDebugAdapterContext; import com.microsoft.java.debug.core.adapter.IHotCodeReplaceProvider; @@ -300,14 +303,21 @@ public void onClassRedefined(Consumer> consumer) { @Override public CompletableFuture> redefineClasses() { + JobHelpers.waitForBuildJobs(10 * 1000); return CompletableFuture.supplyAsync(() -> { List classNames = new ArrayList<>(); + boolean success; synchronized (this) { classNames.addAll(deltaClassNames); - doHotCodeReplace(deltaResources, deltaClassNames); + success = doHotCodeReplace(deltaResources, deltaClassNames); deltaResources.clear(); deltaClassNames.clear(); } + + if (!classNames.isEmpty() && !success) { + throw AdapterUtils.createCompletionException("Failed to hot reload the changed classes", ErrorCode.HCR_FAILURE); + } + return classNames; }); } @@ -325,27 +335,29 @@ private void publishEvent(HotCodeReplaceEvent.EventType type, String message, Ob eventSubject.onNext(new HotCodeReplaceEvent(type, message, data)); } - private void doHotCodeReplace(List resourcesToReplace, List qualifiedNamesToReplace) { + private boolean doHotCodeReplace(List resourcesToReplace, List qualifiedNamesToReplace) { if (context == null || currentDebugSession == null) { - return; + return false; } if (resourcesToReplace == null || qualifiedNamesToReplace == null || qualifiedNamesToReplace.isEmpty() || resourcesToReplace.isEmpty()) { - return; + return false; } filterNotLoadedTypes(resourcesToReplace, qualifiedNamesToReplace); if (qualifiedNamesToReplace.isEmpty()) { - return; + return false; // If none of the changed types are loaded, do nothing. } // Not supported scenario: if (!currentDebugSession.getVM().canRedefineClasses()) { - return; + publishEvent(HotCodeReplaceEvent.EventType.ERROR, "JVM doesn't support hot reload classes"); + return false; } + boolean success = true; publishEvent(HotCodeReplaceEvent.EventType.STARTING, "Start hot code replacement procedure..."); try { @@ -367,6 +379,7 @@ private void doHotCodeReplace(List resourcesToReplace, List q if (containsObsoleteMethods()) { publishEvent(HotCodeReplaceEvent.EventType.ERROR, "JVM contains obsolete methods"); + success = false; } if (currentDebugSession.getVM().canPopFrames() && framesPopped) { @@ -376,11 +389,13 @@ private void doHotCodeReplace(List resourcesToReplace, List q } } catch (DebugException e) { logger.log(Level.SEVERE, "Failed to complete hot code replace: " + e.getMessage(), e); + success = false; } finally { publishEvent(HotCodeReplaceEvent.EventType.END, "Completed hot code replace", qualifiedNamesToReplace); + threadFrameMap.clear(); } - threadFrameMap.clear(); + return success; } private void filterNotLoadedTypes(List resources, List qualifiedNames) { From 8b6bcda24de8705207a38605eb3c5f744a8ef9c5 Mon Sep 17 00:00:00 2001 From: Jinbo Wang Date: Tue, 30 Jun 2020 18:17:18 +0800 Subject: [PATCH 2/2] refine the error message Signed-off-by: Jinbo Wang --- .../handler/HotCodeReplaceHandler.java | 3 +- .../internal/JavaHotCodeReplaceProvider.java | 34 +++++++++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java index aae11a3a6..fa65e978b 100644 --- a/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java +++ b/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java @@ -63,7 +63,8 @@ public CompletableFuture handle(Command command, Arguments arguments, response.body = new Responses.RedefineClassesResponse(classNames.toArray(new String[0])); return CompletableFuture.completedFuture(response); }).exceptionally(ex -> { - response.body = new Responses.RedefineClassesResponse(new String[0], ex.getMessage()); + String errorMessage = ex.getCause() != null ? ex.getCause().getMessage() : ex.getMessage(); + response.body = new Responses.RedefineClassesResponse(new String[0], errorMessage); return response; }); } diff --git a/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java b/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java index cb7d6200f..e1e5c2ec3 100644 --- a/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java +++ b/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java @@ -26,8 +26,10 @@ import java.util.Collections; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; import java.util.logging.Level; @@ -99,9 +101,9 @@ public class JavaHotCodeReplaceProvider implements IHotCodeReplaceProvider, IRes private PublishSubject eventSubject = PublishSubject.create(); - private List deltaResources = new ArrayList<>(); + private Set deltaResources = new LinkedHashSet<>(); - private List deltaClassNames = new ArrayList<>(); + private Set deltaClassNames = new LinkedHashSet<>(); /** * Visitor for resource deltas. @@ -306,16 +308,18 @@ public CompletableFuture> redefineClasses() { JobHelpers.waitForBuildJobs(10 * 1000); return CompletableFuture.supplyAsync(() -> { List classNames = new ArrayList<>(); - boolean success; + List resources = new ArrayList<>(); + String errorMessage = null; synchronized (this) { classNames.addAll(deltaClassNames); - success = doHotCodeReplace(deltaResources, deltaClassNames); + resources.addAll(deltaResources); deltaResources.clear(); deltaClassNames.clear(); + errorMessage = doHotCodeReplace(resources, classNames); } - if (!classNames.isEmpty() && !success) { - throw AdapterUtils.createCompletionException("Failed to hot reload the changed classes", ErrorCode.HCR_FAILURE); + if (!classNames.isEmpty() && errorMessage != null) { + throw AdapterUtils.createCompletionException(errorMessage, ErrorCode.HCR_FAILURE); } return classNames; @@ -335,29 +339,29 @@ private void publishEvent(HotCodeReplaceEvent.EventType type, String message, Ob eventSubject.onNext(new HotCodeReplaceEvent(type, message, data)); } - private boolean doHotCodeReplace(List resourcesToReplace, List qualifiedNamesToReplace) { + private String doHotCodeReplace(List resourcesToReplace, List qualifiedNamesToReplace) { if (context == null || currentDebugSession == null) { - return false; + return null; } if (resourcesToReplace == null || qualifiedNamesToReplace == null || qualifiedNamesToReplace.isEmpty() || resourcesToReplace.isEmpty()) { - return false; + return null; } filterNotLoadedTypes(resourcesToReplace, qualifiedNamesToReplace); if (qualifiedNamesToReplace.isEmpty()) { - return false; + return null; // If none of the changed types are loaded, do nothing. } // Not supported scenario: if (!currentDebugSession.getVM().canRedefineClasses()) { publishEvent(HotCodeReplaceEvent.EventType.ERROR, "JVM doesn't support hot reload classes"); - return false; + return "JVM doesn't support hot reload classes"; } - boolean success = true; + String errorMessage = null; publishEvent(HotCodeReplaceEvent.EventType.STARTING, "Start hot code replacement procedure..."); try { @@ -379,7 +383,7 @@ private boolean doHotCodeReplace(List resourcesToReplace, List resourcesToReplace, List resources, List qualifiedNames) {