From c1c46a3709743d545ccc63397c1081c509903fb9 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 13 Jul 2018 15:20:17 -0700 Subject: [PATCH] RE-using JMXConnectors as much as possible. --- .../boot/app/cli/AbstractSpringBootApp.java | 85 +++++++---- .../boot/app/cli/LocalSpringBootApp.java | 47 +++--- .../boot/app/cli/RemoteSpringBootApp.java | 7 +- .../commons/boot/app/cli/SpringBootApp.java | 4 +- .../vscode/commons/util/ExceptionUtil.java | 5 +- .../util/MemoizingDisposableSupplier.java | 138 ++++++++++++++++++ .../java/handlers/BootJavaHoverProvider.java | 6 +- .../handlers/RemoteRunningAppsProvider.java | 11 +- 8 files changed, 239 insertions(+), 64 deletions(-) create mode 100644 headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingDisposableSupplier.java diff --git a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/AbstractSpringBootApp.java b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/AbstractSpringBootApp.java index be232d5d9..1e3402c94 100644 --- a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/AbstractSpringBootApp.java +++ b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/AbstractSpringBootApp.java @@ -14,7 +14,6 @@ import java.io.File; import java.io.IOException; import java.lang.management.ManagementFactory; import java.lang.management.PlatformManagedObject; -import java.net.MalformedURLException; import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; @@ -40,10 +39,15 @@ import org.springframework.ide.vscode.commons.boot.app.cli.livebean.LiveBeansMod import org.springframework.ide.vscode.commons.boot.app.cli.requestmappings.Boot1xRequestMapping; import org.springframework.ide.vscode.commons.boot.app.cli.requestmappings.RequestMapping; import org.springframework.ide.vscode.commons.boot.app.cli.requestmappings.RequestMappingsParser20; +import org.springframework.ide.vscode.commons.util.MemoizingDisposableSupplier; +import org.springframework.ide.vscode.commons.util.ExceptionUtil; import org.springframework.ide.vscode.commons.util.FuctionWithException; import org.springframework.ide.vscode.commons.util.FunctionWithException; import org.springframework.ide.vscode.commons.util.StringUtil; +import javax.management.remote.JMXConnectorFactory; +import javax.management.remote.JMXServiceURL; + import com.google.common.collect.ImmutableList; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -60,14 +64,14 @@ public abstract class AbstractSpringBootApp implements SpringBootApp { private String jmxMbeanActuatorDomain; // NOTE: Gson-based serialisation replaces the old Jackson ObjectMapper. Not sure if this makes a difference in the long run, but to retain the same output that Jackson Object Mapper - // was generating during serialisatino, some configuration in Gson is required, as the default behaviour of Gson is different than Object Mapper. + // was generating during serialisation, some configuration in Gson is required, as the default behaviour of Gson is different than Object Mapper. // Namely: Object Mapper does not escape Html, whereas Gson does by default (for example // '=' in Gson appears as '\u003d') protected final Gson gson = new GsonBuilder() .disableHtmlEscaping() .create(); - protected abstract JMXServiceURL getJmxUrl() throws MalformedURLException; + protected abstract String getJmxUrl(); @Override public abstract Properties getSystemProperties() throws Exception; @@ -79,11 +83,42 @@ public abstract class AbstractSpringBootApp implements SpringBootApp { @Override public abstract boolean isSpringBootApp(); - protected T withJmxConnector(FunctionWithException doit) throws Exception { - JMXServiceURL serviceUrl = getJmxUrl(); - try (JMXConnector jmxConnector = JMXConnectorFactory.connect(serviceUrl, null)) { - return doit.apply(jmxConnector); + private final MemoizingDisposableSupplier jmxConnector = new MemoizingDisposableSupplier( + //creating jmx connector: + () -> { + String url = getJmxUrl(); + logger.info("Creating JMX connector: "+url); + try { + return JMXConnectorFactory.connect(new JMXServiceURL(url), null); + } catch (Exception e) { + logger.info("Creating JMX connector failed: {}", ExceptionUtil.getMessage(e)); + throw e; + } + }, + //disposing jmx connector: + (connector) -> { + try { + logger.info("Disposing JMX connector: "+getJmxUrl()); + connector.close(); + } catch (IOException e) { + //ignore + } } + ); + + protected T withJmxConnector(FunctionWithException doit) throws Exception { + try { + return doit.apply(jmxConnector.get()); + } catch (Exception e) { + logger.info("Evicting JMX connector {} because of error: {}", getJmxUrl(), ExceptionUtil.getMessage(e)); + jmxConnector.evict(); + throw e; + } + } + + @Override + public void dispose() { + jmxConnector.dispose(); } @Override @@ -247,27 +282,28 @@ public abstract class AbstractSpringBootApp implements SpringBootApp { protected Object getActuatorDataFromAttribute(ObjectName objectName, String attribute) throws Exception { if (objectName != null) { - try { - return withJmxConnector(jmxConnector -> { + return withJmxConnector(jmxConnector -> { + try { MBeanServerConnection connection = jmxConnector.getMBeanServerConnection(); return connection.getAttribute(objectName, "Data"); - }); - } catch (InstanceNotFoundException e) { - } + } catch (InstanceNotFoundException e) { + return null; + } + }); } return null; } protected Object getActuatorDataFromOperation(ObjectName objectName, String operation) throws Exception { if (objectName != null) { - try { - return withJmxConnector(jmxConnector -> { + return withJmxConnector(jmxConnector -> { + try { MBeanServerConnection connection = jmxConnector.getMBeanServerConnection(); return connection.invoke(objectName, operation, null, null); - }); - } - catch (InstanceNotFoundException e) { - } + } catch (InstanceNotFoundException e) { + return null; + } + }); } return null; } @@ -312,11 +348,6 @@ public abstract class AbstractSpringBootApp implements SpringBootApp { @Override public String getJavaCommand() throws Exception { Properties props = getSystemProperties(); - System.err.println(">>>>> sysprops"); - for (Entry e : props.entrySet()) { - System.err.println(e.getKey() +"="+e.getValue()); - } - System.err.println("<<<< sysprops"); return (String) props.get("sun.java.command"); } @@ -324,8 +355,12 @@ public abstract class AbstractSpringBootApp implements SpringBootApp { public String getHost() throws Exception { //TODO: different implementation for cf apps with locally tunnelled // jmx connection? - JMXServiceURL serviceUrl = getJmxUrl(); - return serviceUrl.getHost(); + try { + JMXServiceURL serviceUrl = new JMXServiceURL(getJmxUrl()); + return serviceUrl.getHost(); + } catch (Exception e) { + return "Unknown host"; + } } @Override diff --git a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/LocalSpringBootApp.java b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/LocalSpringBootApp.java index 9a410656a..a83f640a0 100644 --- a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/LocalSpringBootApp.java +++ b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/LocalSpringBootApp.java @@ -11,19 +11,14 @@ package org.springframework.ide.vscode.commons.boot.app.cli; import java.io.IOException; -import java.net.MalformedURLException; import java.util.Collection; import java.util.Map.Entry; import java.util.Properties; -import javax.management.remote.JMXServiceURL; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.ide.vscode.commons.util.CollectorUtil; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import com.sun.tools.attach.AttachNotSupportedException; import com.sun.tools.attach.VirtualMachine; import com.sun.tools.attach.VirtualMachineDescriptor; @@ -42,7 +37,24 @@ public class LocalSpringBootApp extends AbstractSpringBootApp { private Boolean isSpringBootApp; - private final Supplier jmxConnect = Suppliers.memoize(() -> { + private static LocalSpringBootAppCache cache = new LocalSpringBootAppCache(); + + public static Collection getAllRunningJavaApps() throws Exception { + return cache.getAllRunningJavaApps(); + } + + public static Collection getAllRunningSpringApps() throws Exception { + return getAllRunningJavaApps().stream().filter(SpringBootApp::isSpringBootApp).collect(CollectorUtil.toImmutableList()); + } + + public LocalSpringBootApp(VirtualMachineDescriptor vmd) throws AttachNotSupportedException, IOException { + this.vm = VirtualMachine.attach(vmd); + this.vmd = vmd; + } + + + @Override + protected String getJmxUrl() { String address = null; try { address = vm.getAgentProperties().getProperty(LOCAL_CONNECTOR_ADDRESS); @@ -57,22 +69,10 @@ public class LocalSpringBootApp extends AbstractSpringBootApp { } } return address; - }); - - private static LocalSpringBootAppCache cache = new LocalSpringBootAppCache(); - - public static Collection getAllRunningJavaApps() throws Exception { - return cache.getAllRunningJavaApps(); } - public static Collection getAllRunningSpringApps() throws Exception { - return getAllRunningJavaApps().stream().filter(SpringBootApp::isSpringBootApp).collect(CollectorUtil.toImmutableList()); - } - - public LocalSpringBootApp(VirtualMachineDescriptor vmd) throws AttachNotSupportedException, IOException { - this.vmd = vmd; - this.vm = VirtualMachine.attach(vmd); - } +// Supplier jmxConnectUrl = Suppliers.memoize(() -> { +// }); @Override public String getProcessID() { @@ -122,11 +122,6 @@ public class LocalSpringBootApp extends AbstractSpringBootApp { return props.containsKey(key); } - @Override - protected JMXServiceURL getJmxUrl() throws MalformedURLException { - return new JMXServiceURL(jmxConnect.get()); - } - protected boolean contains(String[] cpElements, String element) { for (String cpElement : cpElements) { if (cpElement.contains(element)) { @@ -162,6 +157,7 @@ public class LocalSpringBootApp extends AbstractSpringBootApp { System.out.println("}"); } + @Override public void dispose() { if (vm!=null) { logger.info("SpringBootApp disposed: "+this); @@ -174,5 +170,6 @@ public class LocalSpringBootApp extends AbstractSpringBootApp { if (vmd!=null) { vmd = null; } + super.dispose(); } } diff --git a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/RemoteSpringBootApp.java b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/RemoteSpringBootApp.java index 15ed6135a..c2a41cd9d 100644 --- a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/RemoteSpringBootApp.java +++ b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/RemoteSpringBootApp.java @@ -12,13 +12,10 @@ package org.springframework.ide.vscode.commons.boot.app.cli; import java.io.IOException; import java.lang.management.RuntimeMXBean; -import java.net.MalformedURLException; import java.time.Duration; import java.util.Map.Entry; import java.util.Properties; -import javax.management.remote.JMXServiceURL; - import org.springframework.ide.vscode.commons.util.MemoizingProxy; public class RemoteSpringBootApp extends AbstractSpringBootApp { @@ -30,8 +27,8 @@ public class RemoteSpringBootApp extends AbstractSpringBootApp { } @Override - protected JMXServiceURL getJmxUrl() throws MalformedURLException { - return new JMXServiceURL(jmxUrl); + protected String getJmxUrl() { + return jmxUrl; } @Override diff --git a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/SpringBootApp.java b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/SpringBootApp.java index 821491506..0be1646f3 100644 --- a/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/SpringBootApp.java +++ b/headless-services/commons/commons-boot-app-cli/src/main/java/org/springframework/ide/vscode/commons/boot/app/cli/SpringBootApp.java @@ -18,7 +18,9 @@ import java.util.Properties; import org.springframework.ide.vscode.commons.boot.app.cli.livebean.LiveBeansModel; import org.springframework.ide.vscode.commons.boot.app.cli.requestmappings.RequestMapping; -public interface SpringBootApp { +import reactor.core.Disposable; + +public interface SpringBootApp extends Disposable { String[] getClasspath() throws Exception; String getJavaCommand() throws Exception; diff --git a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java index 97d405bee..908c76ed3 100644 --- a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java +++ b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java @@ -118,7 +118,10 @@ public class ExceptionUtil { ); } - public static RuntimeException unchecked(Exception e) { + public static RuntimeException unchecked(Throwable e) { + if (e instanceof RuntimeException) { + return (RuntimeException)e; + } return new RuntimeException(e); } diff --git a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingDisposableSupplier.java b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingDisposableSupplier.java new file mode 100644 index 000000000..ce9ce7fc1 --- /dev/null +++ b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingDisposableSupplier.java @@ -0,0 +1,138 @@ +/******************************************************************************* + * Copyright (c) 2018 Pivotal, Inc. + * 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: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.commons.util; + +import java.time.Duration; +import java.util.concurrent.Callable; +import java.util.function.Consumer; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.FinalizableReference; +import com.google.common.base.Supplier; + +import reactor.core.Disposable; + +/** + * Wraps a Callable with memoization and logic that allows + * proper cleanup of the memoized value. + *

+ * Both real results and thrown exceptions are memoized. + */ +public class MemoizingDisposableSupplier implements Supplier, Disposable { + + private static Logger logger = LoggerFactory.getLogger(MemoizingDisposableSupplier.class); + + private T value; + private Throwable failure; + private Callable computer; + private Long lastComputed; + + private Long expireExceptionsAfter = null; + private Consumer disposeWith; + + public MemoizingDisposableSupplier(Callable computer, Consumer disposeWith) { + this.computer = computer; + this.disposeWith = disposeWith; + } + + public synchronized void evict() { + T oldValue = value; + if (oldValue!=null) { + disposeWith.accept(oldValue); + } + value = null; + failure = null; + lastComputed = null; + } + + @Override + public void dispose() { + boolean shouldDispose; + synchronized (this) { + shouldDispose = computer!=null; + computer = null; + } + if (shouldDispose) { + if (disposeWith!=null && value!=null) { + disposeWith.accept(value); + } + value = null; + failure = null; + lastComputed = null; + disposeWith = null; + } + } + + @Override + public synchronized T get() { + Assert.isLegal(!isDisposed()); + if (shouldCompute()) { + lastComputed = System.currentTimeMillis(); + T oldValue = value; + if (oldValue instanceof Disposable) { + ((Disposable) oldValue).dispose(); + } + try { + value = computer.call(); + failure = null; + } catch (Throwable e) { + value = null; + failure = e; + } + } + if (failure!=null) { + throw ExceptionUtil.unchecked(failure); + } else { + return value; + } + } + + @Override + public boolean isDisposed() { + return computer==null; + } + + private boolean shouldCompute() { + if (lastComputed==null) { + //Never computed + return true; + } else { + //Computed before... should check expiration + if (failure!=null) { + // cached result is a exception + return expireExceptionsAfter!=null && + System.currentTimeMillis() - lastComputed >= expireExceptionsAfter; + } else { + // cached result is a normal value + return false; //for now normal values never expire. + } + } + } + + public MemoizingDisposableSupplier expireExceptions(Duration after) { + this.expireExceptionsAfter = after.toMillis(); + return this; + } + + @Override + protected void finalize() throws Throwable { + //TODO: consider removing this method when we have confidence we have no leaks that need cleaning up. + // Using finalizers is expensive so should be avoided. This is only here as a temporary fail-safe. + // Proper cleanup should be implemented and logger.error below should never be reached. + if (!isDisposed()) { + logger.error("Leaked Disposable detected: "+this); + this.dispose(); + } + } + +} diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java index cb359188c..0c6ebe255 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java @@ -225,17 +225,17 @@ public class BootJavaHoverProvider implements HoverHandler { private Hover provideHoverForAnnotation(ASTNode exactNode, Annotation annotation, int offset, TextDocument doc, IJavaProject project) { ITypeBinding type = annotation.resolveTypeBinding(); if (type != null) { - logger.info("Hover requested for "+type.getName()); + logger.debug("Hover requested for "+type.getName()); SpringBootApp[] runningApps = getRunningSpringApps(project); if (runningApps.length > 0) { for (HoverProvider provider : this.hoverProviders.get(type)) { Hover hover = provider.provideHover(exactNode, annotation, type, offset, doc, project, runningApps); if (hover!=null) { - logger.info("Hover found: "+hover); + logger.debug("Hover found: "+hover); //TODO: compose multiple hovers somehow instead of just returning the first one? return hover; } - logger.info("NO Hover!"); + logger.debug("NO Hover!"); } //Only reaching here if we didn't get a hover. if (!hasActuatorDependency(project)) { diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/RemoteRunningAppsProvider.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/RemoteRunningAppsProvider.java index 9feb58c2c..00cdd81dd 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/RemoteRunningAppsProvider.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/RemoteRunningAppsProvider.java @@ -14,6 +14,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import org.slf4j.Logger; @@ -47,12 +48,14 @@ public class RemoteRunningAppsProvider implements RunningAppProvider { Set urls = settings.getStringSet("boot-java", "remote-apps"); { //Remove obsolete apps... - Iterator keys = remoteAppByUrl.keySet().iterator(); - while (keys.hasNext()) { - String key = keys.next(); + Iterator> entries = remoteAppByUrl.entrySet().iterator(); + while (entries.hasNext()) { + Entry entry = entries.next(); + String key = entry.getKey(); if (!urls.contains(key)) { logger.debug("Removing RemoteSpringBootApp: "+key); - keys.remove(); + entries.remove(); + entry.getValue().dispose(); } } }