4714674: JEditorPane.setPage(url) blocks AWT thread when HTTP protocol is used
Summary: Both POST and GET can now be processed asynchronously; PageLoader refactored
Reviewed-by: gsm
--- a/jdk/src/share/classes/javax/swing/JEditorPane.java Fri Feb 29 20:04:01 2008 -0800
+++ b/jdk/src/share/classes/javax/swing/JEditorPane.java Thu Apr 03 16:41:43 2008 +0400
@@ -429,9 +429,8 @@
// different url or POST method, load the new content
int p = getAsynchronousLoadPriority(getDocument());
- if ((postData == null) || (p < 0)) {
- // Either we do not have POST data, or should submit the data
- // synchronously.
+ if (p < 0) {
+ // open stream synchronously
InputStream in = getStream(page);
if (kit != null) {
Document doc = initializeModel(kit, page);
@@ -440,22 +439,13 @@
// view notifications slowing it down (i.e. best synchronous
// behavior) or set the model and start to feed it on a separate
// thread (best asynchronous behavior).
- synchronized(this) {
- if (loading != null) {
- // we are loading asynchronously, so we need to cancel
- // the old stream.
- loading.cancel();
- loading = null;
- }
- }
p = getAsynchronousLoadPriority(doc);
if (p >= 0) {
// load asynchronously
setDocument(doc);
synchronized(this) {
- loading = new PageStream(in);
- Thread pl = new PageLoader(doc, loading, p, loaded, page);
- pl.start();
+ pageLoader = new PageLoader(doc, in, loaded, page);
+ pageLoader.execute();
}
return;
}
@@ -464,11 +454,15 @@
reloaded = true;
}
} else {
- // We have POST data and should send it asynchronously.
- // Send (and subsequentally read) data in separate thread.
+ // we may need to cancel background loading
+ if (pageLoader != null) {
+ pageLoader.cancel(true);
+ }
+
+ // Do everything in a background thread.
// Model initialization is deferred to that thread, too.
- Thread pl = new PageLoader(null, null, p, loaded, page);
- pl.start();
+ pageLoader = new PageLoader(null, null, loaded, page);
+ pageLoader.execute();
return;
}
}
@@ -604,44 +598,38 @@
/**
- * Thread to load a stream into the text document model.
+ * Loads a stream into the text document model.
*/
- class PageLoader extends Thread {
+ class PageLoader extends SwingWorker<URL, Object> {
/**
* Construct an asynchronous page loader.
*/
- PageLoader(Document doc, InputStream in, int priority, URL old,
- URL page) {
- setPriority(priority);
+ PageLoader(Document doc, InputStream in, URL old, URL page) {
this.in = in;
this.old = old;
this.page = page;
this.doc = doc;
}
- boolean pageLoaded = false;
-
/**
* Try to load the document, then scroll the view
* to the reference (if specified). When done, fire
* a page property change event.
*/
- public void run() {
+ protected URL doInBackground() {
+ boolean pageLoaded = false;
try {
if (in == null) {
in = getStream(page);
if (kit == null) {
// We received document of unknown content type.
- UIManager.getLookAndFeel().provideErrorFeedback(
- JEditorPane.this);
- return;
- }
- // Access to <code>loading</code> should be synchronized.
- synchronized(JEditorPane.this) {
- in = loading = new PageStream(in);
+ UIManager.getLookAndFeel().
+ provideErrorFeedback(JEditorPane.this);
+ return old;
}
}
+
if (doc == null) {
try {
SwingUtilities.invokeAndWait(new Runnable() {
@@ -653,11 +641,11 @@
} catch (InvocationTargetException ex) {
UIManager.getLookAndFeel().provideErrorFeedback(
JEditorPane.this);
- return;
+ return old;
} catch (InterruptedException ex) {
UIManager.getLookAndFeel().provideErrorFeedback(
JEditorPane.this);
- return;
+ return old;
}
}
@@ -682,16 +670,14 @@
} catch (IOException ioe) {
UIManager.getLookAndFeel().provideErrorFeedback(JEditorPane.this);
} finally {
- synchronized(JEditorPane.this) {
- loading = null;
+ if (pageLoaded) {
+ SwingUtilities.invokeLater(new Runnable() {
+ public void run() {
+ JEditorPane.this.firePropertyChange("page", old, page);
+ }
+ });
}
- SwingUtilities.invokeLater(new Runnable() {
- public void run() {
- if (pageLoaded) {
- firePropertyChange("page", old, page);
- }
- }
- });
+ return (pageLoaded ? page : old);
}
}
@@ -718,51 +704,6 @@
Document doc;
}
- static class PageStream extends FilterInputStream {
-
- boolean canceled;
-
- public PageStream(InputStream i) {
- super(i);
- canceled = false;
- }
-
- /**
- * Cancel the loading of the stream by throwing
- * an IOException on the next request.
- */
- public synchronized void cancel() {
- canceled = true;
- }
-
- protected synchronized void checkCanceled() throws IOException {
- if (canceled) {
- throw new IOException("page canceled");
- }
- }
-
- public int read() throws IOException {
- checkCanceled();
- return super.read();
- }
-
- public long skip(long n) throws IOException {
- checkCanceled();
- return super.skip(n);
- }
-
- public int available() throws IOException {
- checkCanceled();
- return super.available();
- }
-
- public void reset() throws IOException {
- checkCanceled();
- super.reset();
- }
-
- }
-
/**
* Fetches a stream for the given URL, which is about to
* be loaded by the <code>setPage</code> method. By
@@ -1573,11 +1514,7 @@
// --- variables ---------------------------------------
- /**
- * Stream currently loading asynchronously (potentially cancelable).
- * Access to this variable should be synchronized.
- */
- PageStream loading;
+ private SwingWorker<URL, Object> pageLoader;
/**
* Current content binding of the editor.
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/javax/swing/JEditorPane/bug4714674.java Thu Apr 03 16:41:43 2008 +0400
@@ -0,0 +1,124 @@
+/* @test
+ @bug 4714674
+ @summary Tests that JEditorPane opens HTTP connection asynchronously
+ @author Peter Zhelezniakov
+ @run main bug4714674
+*/
+
+import javax.swing.*;
+
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.concurrent.Executors;
+
+
+public class bug4714674 {
+
+ public static void main(String[] args) throws Exception {
+ new bug4714674().test();
+ }
+
+ private void test() throws Exception {
+ final DeafServer server = new DeafServer();
+ final String baseURL = "http://localhost:" + server.getPort() + "/";
+
+ SwingUtilities.invokeLater(new Runnable() {
+ public void run() {
+ try {
+ JEditorPane pane = new JEditorPane();
+ ((javax.swing.text.AbstractDocument)pane.getDocument()).
+ setAsynchronousLoadPriority(1);
+
+ // this will block EDT unless 4714674 is fixed
+ pane.setPage(baseURL);
+ } catch (IOException e) {
+ // should not happen
+ throw new Error(e);
+ }
+ }
+ });
+
+ // if 4714674 is fixed, this executes before connection times out
+ SwingUtilities.invokeLater(new Runnable() {
+ public void run() {
+ synchronized (server) {
+ server.wakeup = true;
+ server.notifyAll();
+ }
+ pass();
+ }
+ });
+
+ // wait, then check test status
+ try {
+ Thread.sleep(5000);
+ if (!passed()) {
+ throw new RuntimeException("Failed: EDT was blocked");
+ }
+ } finally {
+ server.destroy();
+ }
+ }
+
+ private boolean passed = false;
+
+ private synchronized boolean passed() {
+ return passed;
+ }
+
+ private synchronized void pass() {
+ passed = true;
+ }
+}
+
+/**
+ * A "deaf" HTTP server that does not respond to requests.
+ */
+class DeafServer {
+ HttpServer server;
+ boolean wakeup = false;
+
+ /**
+ * Create and start the HTTP server.
+ */
+ public DeafServer() throws IOException {
+ InetSocketAddress addr = new InetSocketAddress(0);
+ server = HttpServer.create(addr, 0);
+ HttpHandler handler = new DeafHandler();
+ server.createContext("/", handler);
+ server.setExecutor(Executors.newCachedThreadPool());
+ server.start();
+ }
+
+ /**
+ * Stop server without any delay.
+ */
+ public void destroy() {
+ server.stop(0);
+ }
+
+ /**
+ * Return actual server port number, useful for constructing request URIs.
+ */
+ public int getPort() {
+ return server.getAddress().getPort();
+ }
+
+
+ class DeafHandler implements HttpHandler {
+ public void handle(HttpExchange r) throws IOException {
+ synchronized (DeafServer.this) {
+ while (! wakeup) {
+ try {
+ DeafServer.this.wait();
+ } catch (InterruptedException e) {
+ // just wait again
+ }
+ }
+ }
+ }
+ }
+}