This is an automated email from the ASF dual-hosted git repository. jtulach pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/netbeans-html4j.git
commit a458fd4bb91836518d9a6a831eb34ff2ea9b888d Author: Jaroslav Tulach <jaroslav.tul...@apidesign.org> AuthorDate: Sat May 4 07:30:13 2019 +0200 Assert that unused FX presenters can be GCed --- .../html/boot/impl/JavaScriptProcesor.java | 7 +- .../main/java/org/netbeans/html/boot/spi/Fn.java | 127 ++++++++++++--------- ko4j/pom.xml | 6 + .../java/org/netbeans/html/ko4j/CacheObjs.java | 8 +- .../main/java/org/netbeans/html/ko4j/Knockout.java | 15 ++- .../main/java/org/netbeans/html/ko4j/MapObjs.java | 50 +++++--- .../org/netbeans/html/ko4j/DoubleViewTest.java | 41 ++++++- 7 files changed, 174 insertions(+), 80 deletions(-) diff --git a/boot/src/main/java/org/netbeans/html/boot/impl/JavaScriptProcesor.java b/boot/src/main/java/org/netbeans/html/boot/impl/JavaScriptProcesor.java index 7e13130..3f259fa 100644 --- a/boot/src/main/java/org/netbeans/html/boot/impl/JavaScriptProcesor.java +++ b/boot/src/main/java/org/netbeans/html/boot/impl/JavaScriptProcesor.java @@ -400,16 +400,16 @@ public final class JavaScriptProcesor extends AbstractProcessor { source.append("package ").append(pkgName).append(";\n"); source.append("public final class $JsCallbacks$ {\n"); source.append(" static final $JsCallbacks$ VM = new $JsCallbacks$(null);\n"); - source.append(" private final org.netbeans.html.boot.spi.Fn.Presenter p;\n"); + source.append(" private final java.lang.ref.Reference<org.netbeans.html.boot.spi.Fn.Presenter> ref;\n"); source.append(" private $JsCallbacks$ next;\n"); source.append(" private $JsCallbacks$(org.netbeans.html.boot.spi.Fn.Presenter p) {\n"); - source.append(" this.p = p;\n"); + source.append(" this.ref = new java.lang.ref.WeakReference<org.netbeans.html.boot.spi.Fn.Presenter>(p);\n"); source.append(" }\n"); source.append(" synchronized final $JsCallbacks$ current() {\n"); source.append(" org.netbeans.html.boot.spi.Fn.Presenter now = org.netbeans.html.boot.spi.Fn.activePresenter();\n"); source.append(" $JsCallbacks$ thiz = this;\n"); source.append(" for (;;) {\n"); - source.append(" if (now == thiz.p) return thiz;\n"); + source.append(" if (now == thiz.ref.get()) return thiz;\n"); source.append(" if (thiz.next == null) {\n"); source.append(" return thiz.next = new $JsCallbacks$(now);\n"); source.append(" }\n"); @@ -486,6 +486,7 @@ public final class JavaScriptProcesor extends AbstractProcessor { sep = ", "; } source.append(") throws Throwable {\n"); + source.append(" org.netbeans.html.boot.spi.Fn.Presenter p = ref.get(); \n"); source.append(convert); if (useTryResources()) { source.append(" try (java.io.Closeable a = org.netbeans.html.boot.spi.Fn.activate(p)) { \n"); diff --git a/boot/src/main/java/org/netbeans/html/boot/spi/Fn.java b/boot/src/main/java/org/netbeans/html/boot/spi/Fn.java index e70ad91..3dc4e30 100644 --- a/boot/src/main/java/org/netbeans/html/boot/spi/Fn.java +++ b/boot/src/main/java/org/netbeans/html/boot/spi/Fn.java @@ -23,6 +23,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; +import java.lang.ref.Reference; +import java.lang.ref.WeakReference; import java.net.URL; import java.util.HashMap; import java.util.HashSet; @@ -38,8 +40,7 @@ import org.netbeans.html.boot.impl.FnContext; * @author Jaroslav Tulach */ public abstract class Fn { - private static Map<String, Set<Presenter>> LOADED; - private final Presenter presenter; + private final Reference<Presenter> presenter; /** * @deprecated Ineffective as of 0.6. @@ -57,7 +58,7 @@ public abstract class Fn { * @since 0.6 */ protected Fn(Presenter presenter) { - this.presenter = presenter; + this.presenter = new WeakReference<Presenter>(presenter); } /** True, if currently active presenter is the same as presenter this @@ -66,7 +67,7 @@ public abstract class Fn { * @return true, if proper presenter is used */ public final boolean isValid() { - return presenter != null && FnContext.currentPresenter(false) == presenter; + return FnContext.currentPresenter(false) == presenter.get(); } /** Helper method to check if the provided instance is valid function. @@ -144,52 +145,7 @@ public abstract class Fn { if (fn == null) { return null; } - return new Fn(fn.presenter()) { - @Override - public Object invoke(Object thiz, Object... args) throws Exception { - loadResource(); - return fn.invoke(thiz, args); - } - - @Override - public void invokeLater(Object thiz, Object... args) throws Exception { - loadResource(); - fn.invokeLater(thiz, args); - } - - private void loadResource() throws Exception { - Presenter p = presenter(); - if (p == null) { - p = FnContext.currentPresenter(false); - } - if (p != null) { - if (LOADED == null) { - LOADED = new HashMap<String, Set<Presenter>>(); - } - Set<Presenter> there = LOADED.get(resource); - if (there == null) { - there = new HashSet<Presenter>(); - LOADED.put(resource, there); - } - if (there.add(p)) { - final ClassLoader l = caller.getClassLoader(); - InputStream is = l.getResourceAsStream(resource); - if (is == null && resource.startsWith("/")) { - is = l.getResourceAsStream(resource.substring(1)); - } - if (is == null) { - throw new IOException("Cannot find " + resource + " in " + l); - } - try { - InputStreamReader r = new InputStreamReader(is, "UTF-8"); - p.loadScript(r); - } finally { - is.close(); - } - } - } - } - }; + return new Preload(fn.presenter(), fn, resource, caller); } @@ -251,7 +207,7 @@ public abstract class Fn { * @since 0.7 */ protected final Presenter presenter() { - return presenter; + return presenter.get(); } /** The representation of a <em>presenter</em> - usually a browser window. @@ -363,4 +319,73 @@ public abstract class Fn { */ public Fn defineFn(String code, String[] names, boolean[] keepAlive); } + + private static class Preload extends Fn { + private static Map<String, Set<Reference<Presenter>>> LOADED; + private final Fn fn; + private final String resource; + private final Class<?> caller; + + Preload(Presenter presenter, Fn fn, String resource, Class<?> caller) { + super(presenter); + this.fn = fn; + this.resource = resource; + this.caller = caller; + } + + @Override + public Object invoke(Object thiz, Object... args) throws Exception { + loadResource(); + return fn.invoke(thiz, args); + } + + @Override + public void invokeLater(Object thiz, Object... args) throws Exception { + loadResource(); + fn.invokeLater(thiz, args); + } + + private void loadResource() throws Exception { + Reference<Presenter> ref = super.presenter; + if (ref == null) { + ref = new WeakReference<Fn.Presenter>(FnContext.currentPresenter(false)); + } + Fn.Presenter realPresenter = ref == null ? null : ref.get(); + if (realPresenter != null) { + if (LOADED == null) { + LOADED = new HashMap<String, Set<Reference<Presenter>>>(); + } + Set<Reference<Presenter>> there = LOADED.get(resource); + if (there == null) { + there = new HashSet<Reference<Fn.Presenter>>(); + LOADED.put(resource, there); + } + if (addNewRef(there, ref)) { + final ClassLoader l = caller.getClassLoader(); + InputStream is = l.getResourceAsStream(resource); + if (is == null && resource.startsWith("/")) { + is = l.getResourceAsStream(resource.substring(1)); + } + if (is == null) { + throw new IOException("Cannot find " + resource + " in " + l); + } + try { + InputStreamReader r = new InputStreamReader(is, "UTF-8"); + realPresenter.loadScript(r); + } finally { + is.close(); + } + } + } + } + + private static synchronized boolean addNewRef(Set<Reference<Presenter>> set, Reference<Presenter> ref) { + for (Reference<Presenter> r : set) { + if (r.get() == ref.get()) { + return false; + } + } + return set.add(ref); + } + } } diff --git a/ko4j/pom.xml b/ko4j/pom.xml index fbe56bb..1d8b9d1 100644 --- a/ko4j/pom.xml +++ b/ko4j/pom.xml @@ -130,6 +130,12 @@ <artifactId>javax.servlet-api</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>org.netbeans.api</groupId> + <artifactId>org-netbeans-modules-nbjunit</artifactId> + <version>${netbeans.version}</version> + <scope>test</scope> + </dependency> </dependencies> <description>Binds net.java.html.json APIs together with knockout.js</description> </project> diff --git a/ko4j/src/main/java/org/netbeans/html/ko4j/CacheObjs.java b/ko4j/src/main/java/org/netbeans/html/ko4j/CacheObjs.java index 4da77c2..74fe85d 100644 --- a/ko4j/src/main/java/org/netbeans/html/ko4j/CacheObjs.java +++ b/ko4j/src/main/java/org/netbeans/html/ko4j/CacheObjs.java @@ -19,24 +19,26 @@ package org.netbeans.html.ko4j; +import java.lang.ref.Reference; +import java.lang.ref.WeakReference; import org.netbeans.html.boot.spi.Fn; final class CacheObjs { /* both @GuardedBy CacheObjs.class */ private static CacheObjs[] list = new CacheObjs[16]; private static int listAt = 0; - private final Fn.Presenter ref; + private final Reference<Fn.Presenter> ref; /* both @GuardedBy presenter single threaded access */ private Object[] jsObjects; private int jsIndex; private CacheObjs(Fn.Presenter p) { - this.ref = p; + this.ref = new WeakReference<Fn.Presenter>(p); } Fn.Presenter get() { - return ref; + return ref.get(); } static synchronized CacheObjs find(Fn.Presenter key) { diff --git a/ko4j/src/main/java/org/netbeans/html/ko4j/Knockout.java b/ko4j/src/main/java/org/netbeans/html/ko4j/Knockout.java index 912d12d..e95ff87 100644 --- a/ko4j/src/main/java/org/netbeans/html/ko4j/Knockout.java +++ b/ko4j/src/main/java/org/netbeans/html/ko4j/Knockout.java @@ -20,6 +20,7 @@ package org.netbeans.html.ko4j; import java.io.Closeable; import java.io.IOException; +import java.lang.ref.Reference; import java.util.HashMap; import java.util.Map; import java.util.concurrent.Executor; @@ -150,10 +151,22 @@ final class Knockout { funcs[index].call(data, ev); } + private static Fn.Presenter getPresenter(Object obj) { + if (obj instanceof Fn.Presenter) { + return (Fn.Presenter) obj; + } else { + if (obj == null) { + return null; + } else { + return (Fn.Presenter) ((Reference<?>)obj).get(); + } + } + } + final void valueHasMutated(final String propertyName, Object oldValue, Object newValue) { Object[] all = MapObjs.toArray(objs); for (int i = 0; i < all.length; i += 2) { - Fn.Presenter p = (Fn.Presenter) all[i]; + Fn.Presenter p = getPresenter(all[i]); final Object o = all[i + 1]; if (p != Fn.activePresenter()) { if (p instanceof Executor) { diff --git a/ko4j/src/main/java/org/netbeans/html/ko4j/MapObjs.java b/ko4j/src/main/java/org/netbeans/html/ko4j/MapObjs.java index 1f3a998..bf9d998 100644 --- a/ko4j/src/main/java/org/netbeans/html/ko4j/MapObjs.java +++ b/ko4j/src/main/java/org/netbeans/html/ko4j/MapObjs.java @@ -19,12 +19,14 @@ package org.netbeans.html.ko4j; +import java.lang.ref.Reference; +import java.lang.ref.WeakReference; import java.util.List; import net.java.html.json.Models; import org.netbeans.html.boot.spi.Fn; final class MapObjs { - private static Object onlyPresenter; + private static Reference<Fn.Presenter> onlyPresenter; private static boolean usePresenter; static { @@ -32,7 +34,7 @@ final class MapObjs { } static void reset() { - onlyPresenter = null; + setOnlyPresenter(null); usePresenter = true; } @@ -43,33 +45,33 @@ final class MapObjs { } - static Object put(Object now, Fn.Presenter key, Object js) { + synchronized static Object put(Object now, Fn.Presenter key, Object js) { if (now instanceof MapObjs) { return ((MapObjs)now).put(key, js); } else { if (usePresenter) { - if (onlyPresenter == null) { - onlyPresenter = key; + if (getOnlyPresenter() == null) { + setOnlyPresenter(key); return js; - } else if (onlyPresenter == key) { + } else if (getOnlyPresenter() == key) { return js; } else { usePresenter = false; } } if (now == null) { - return new MapObjs(key, js); + return new MapObjs(new WeakReference<Fn.Presenter>(key), js); } else { - return new MapObjs(onlyPresenter, now, key, js); + return new MapObjs(onlyPresenter, now, new WeakReference<Fn.Presenter>(key), js); } } } - static Object get(Object now, Fn.Presenter key) { + synchronized static Object get(Object now, Fn.Presenter key) { if (now instanceof MapObjs) { return ((MapObjs)now).get(key); } - return key == onlyPresenter ? now : null; + return key == getOnlyPresenter() ? now : null; } static Object[] remove(Object now, Fn.Presenter key) { @@ -79,28 +81,36 @@ final class MapObjs { return new Object[] { now, null }; } - static Object[] toArray(Object now) { + synchronized static Object[] toArray(Object now) { if (now instanceof MapObjs) { return ((MapObjs) now).all.toArray(); } - return new Object[] { onlyPresenter, now }; + return new Object[] { getOnlyPresenter(), now }; } private Object put(Fn.Presenter key, Object js) { for (int i = 0; i < all.size(); i += 2) { - if (all.get(i) == key) { + if (isSameKey(i, key)) { all.set(i + 1, js); return this; } } - all.add(key); + all.add(new WeakReference<Fn.Presenter>(key)); all.add(js); return this; } + boolean isSameKey(int index, Fn.Presenter key) { + Object at = all.get(index); + if (at instanceof Reference<?>) { + at = ((Reference<?>)at).get(); + } + return at == key; + } + private Object get(Fn.Presenter key) { for (int i = 0; i < all.size(); i += 2) { - if (all.get(i) == key) { + if (isSameKey(i, key)) { return all.get(i + 1); } } @@ -109,10 +119,18 @@ final class MapObjs { private Object[] remove(Fn.Presenter key) { for (int i = 0; i < all.size(); i += 2) { - if (all.get(i) == key) { + if (isSameKey(i, key)) { return new Object[] { all.get(i + 1), this }; } } return new Object[] { null, this }; } + + private static Fn.Presenter getOnlyPresenter() { + return onlyPresenter.get(); + } + + private static void setOnlyPresenter(Fn.Presenter aOnlyPresenter) { + onlyPresenter = new WeakReference<Fn.Presenter>(aOnlyPresenter); + } } diff --git a/ko4j/src/test/java/org/netbeans/html/ko4j/DoubleViewTest.java b/ko4j/src/test/java/org/netbeans/html/ko4j/DoubleViewTest.java index d2ed578..9f49d84 100644 --- a/ko4j/src/test/java/org/netbeans/html/ko4j/DoubleViewTest.java +++ b/ko4j/src/test/java/org/netbeans/html/ko4j/DoubleViewTest.java @@ -19,11 +19,15 @@ package org.netbeans.html.ko4j; import java.awt.FlowLayout; +import java.lang.ref.Reference; +import java.lang.ref.WeakReference; import java.net.URL; import java.util.concurrent.CountDownLatch; import javafx.application.Platform; import javafx.embed.swing.JFXPanel; +import javafx.scene.Parent; import javafx.scene.Scene; +import javafx.scene.control.Label; import javafx.scene.layout.BorderPane; import javafx.scene.web.WebView; import javax.swing.JFrame; @@ -31,6 +35,7 @@ import net.java.html.boot.fx.FXBrowsers; import net.java.html.json.Function; import net.java.html.json.Model; import net.java.html.json.Property; +import org.netbeans.junit.NbTestCase; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; import org.testng.annotations.AfterMethod; @@ -57,6 +62,7 @@ public class DoubleViewTest { private WebView view2; private static final StringBuffer LOG = new StringBuffer(); + private JFrame frame; @BeforeMethod public void initializeViews() throws Exception { @@ -104,12 +110,12 @@ public class DoubleViewTest { view1 = displayWebView(panel); view2 = displayWebView(p2); - JFrame f = new JFrame(); - f.getContentPane().setLayout(new FlowLayout()); - f.getContentPane().add(panel); - f.getContentPane().add(p2); - f.pack(); - f.setVisible(true); + frame = new JFrame(); + frame.getContentPane().setLayout(new FlowLayout()); + frame.getContentPane().add(panel); + frame.getContentPane().add(p2); + frame.pack(); + frame.setVisible(true); } private WebView displayWebView(JFXPanel panel) { @@ -152,6 +158,29 @@ public class DoubleViewTest { @AfterMethod public void waitABit() throws Exception { + final CountDownLatch cdl = new CountDownLatch(1); + Platform.runLater(() -> { + Parent p1 = view1.getParent(); + ((BorderPane)p1).setCenter(new Label("Searching for GC root")); + Parent p2 = view2.getParent(); + ((BorderPane)p2).setCenter(new Label("Searching for GC root")); + cdl.countDown(); + }); + cdl.await(); + + Reference<?> r1 = new WeakReference<>(view1); + Reference<?> r2 = new WeakReference<>(view2); + + view1 = null; + view2 = null; + + NbTestCase.assertGC("Clearing reference 1", r1); + NbTestCase.assertGC("Clearing reference 2", r2); + + Platform.runLater(() -> { + Platform.setImplicitExit(false); + frame.dispose(); + }); } private void assertMessages(String msg, WebView v, String expected) { --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@netbeans.apache.org For additional commands, e-mail: commits-h...@netbeans.apache.org For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists