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

8242361: JavaFX Web View crashes with Segmentation Fault, when HTML contains Data-URIs #360

Closed
wants to merge 6 commits into from
@@ -48,7 +48,7 @@ void initializeMainThreadPlatform()
// Initialize the class reference and methodids for the MainThread. The
// initilization has to be done from a context where the class
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@arun-Joseph

arun-Joseph Dec 16, 2020
Member

initilization -> initialization

// com.sun.webkit.MainThread is accessible. When
// scheduleDispatchFunctionsOnMainThread is invoced, the system class loader
// scheduleDispatchFunctionsOnMainThread is invoked, the system class loader
// would be used to locate the class, which fails if the JavaFX modules are
// not loaded from the boot module layer.
//
@@ -62,6 +62,7 @@ void initializeMainThreadPlatform()
// WebPage will be used by FindClass.
//
// WTF::initializeMainThread has a guard, so that initialization is only run
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@kevinrushforth

kevinrushforth Dec 14, 2020
Member

I guess you meant to say "is only run once"?

// once

AttachThreadAsNonDaemonToJavaEnv autoAttach;
JNIEnv* env = autoAttach.env();
@@ -37,7 +37,7 @@
* @summary Check if webkit main thread <-> java integration works correctly
*/
public class MainThreadTest {
@Test
@Test (timeout = 15000)
public void testMainThreadDoesNotSegfault() throws Exception {
// This is an indirect test of the webkit main thread <-> java
// integration. It was observed, that using a data-url caused the
@@ -57,7 +57,7 @@ public void testMainThreadDoesNotSegfault() throws Exception {

final List<String> cmd = asList(
workerJavaCmd,
"-cp",appModulePath + "/mymod",
"-cp", appModulePath + "/mymod",
"-Djava.library.path=" + javaLibraryPath,
"-Dmodule.path=" + appModulePath + "/mymod" + File.pathSeparator + workerModulePath,
"myapp7.DataUrlWithModuleLayerLauncher"
@@ -29,38 +29,44 @@
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.concurrent.Worker.State;
import javafx.scene.Scene;
import javafx.scene.layout.BorderPane;
import javafx.scene.web.WebView;
import javafx.stage.Stage;

public class DataUrlWithModuleLayer extends Application {
public static final int ERROR_OK = 0;
public static final int ERROR_ASSUMPTION_VIOLATED = 1;
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@kevinrushforth

kevinrushforth Dec 15, 2020
Member

We usually leave 1 unused, since that is what Process.waitFor returns if the application fails to launch (in case that happens it will be easier for someone to diagnose).

public static final int ERROR_TIMEOUT = 2;
public static final int ERROR_TITLE_NOT_UPDATED = 3;

@Override
public void start(Stage primaryStage) throws Exception {
Module module = Application.class.getModule();

if (module == null) {
System.err.println("Failure: Module for Application not found");
System.exit(1);
System.exit(ERROR_ASSUMPTION_VIOLATED);
}

if (! module.isNamed()) {
System.err.println("Failure: Expected named module");
System.exit(1);
System.exit(ERROR_ASSUMPTION_VIOLATED);
}

ModuleDescriptor moduleDesc = module.getDescriptor();

if (moduleDesc.isAutomatic()) {
System.err.println("Failure: Automatic module found");
System.exit(1);
System.exit(ERROR_ASSUMPTION_VIOLATED);
}

if (moduleDesc.isOpen()) {
System.err.println("Failure: Open module found");
System.exit(1);
System.exit(ERROR_ASSUMPTION_VIOLATED);
}

BorderPane root = new BorderPane();
@@ -85,31 +91,24 @@ public void start(Stage primaryStage) throws Exception {
+ "<script src=\"data:application/javascript;base64," + checkJSEncoded + "\"></script>"
+ "</body>"
+ "</html>";

webview.getEngine().getLoadWorker().stateProperty().addListener(
new ChangeListener<State>() {
public void changed(ObservableValue ov, State oldState, State newState) {
String title = webview.getEngine().getTitle();
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@kevinrushforth

kevinrushforth Dec 15, 2020
Member

Minor: you might want to move this inside the test for SUCCEEDED since it isn't valid until then.

if (newState == State.SUCCEEDED) {
if ("Executed".equals(title)) {
System.exit(ERROR_OK);
} else {
System.exit(ERROR_TITLE_NOT_UPDATED);
}
}
}
});
webview.getEngine().loadContent(script);

primaryStage.setScene(scene);
primaryStage.setWidth(1024);
primaryStage.setHeight(768);
primaryStage.show();

new Thread() {
@Override
public void run() {
while (true) {
Platform.runLater(() -> {
String title = webview.getEngine().getTitle();
if("Executed".equals(title)) {
System.exit(0);
}
});
try {
Thread.sleep(500);
} catch (InterruptedException ex) {
break;
}
}
}
}.start();
}
}
@@ -38,6 +38,19 @@
public class DataUrlWithModuleLayerLauncher {

public static void main(String[] args) throws Exception {
// Install safeguard to ensure this application is terminated
new Thread() {
@Override
public void run() {
try {
Thread.sleep(15000);
} catch (InterruptedException ex) {
// Ok, lets exit early
}
System.exit(DataUrlWithModuleLayer.ERROR_TIMEOUT);
}
}.start();

/*
* Setup a module layer for OpenJFX and the test class
*/
ProTip! Use n and p to navigate between commits in a pull request.