Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
WebKit has never used Nashorn so this is unrelated to the impending Nashorn removal. -- Kevin On 3/23/2020 5:49 AM, Eric Bresie wrote: This may be a little off topic but given this is a Javascript concern, is this (or will this) be impacted by Nashhorns depreciation (as of jdk11 I believe) and potential removal at some point? I see reference to Netscape JObject do maybe that circumvents some of that but does this need to eventually transition to graalvm or other means? Eric Bresie ebre...@gmail.com On March 10, 2020 at 6:22:01 AM CDT, Ajit Ghaisas wrote: On Fri, 6 Mar 2020 19:53:04 GMT, Kevin Rushforth wrote: Please ignore this comment as well, it is also for debugging issues with Skara and mailman :email: @aghaisas can you review this? I executed these tests without this patch. They did not hang on my Macbook. I applied patch and executed the tests. They continue to pass. I see the merit of this change to make tests predictable. +1. - PR: https://git.openjdk.java.net/jfx/pull/134
Re: Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
This may be a little off topic but given this is a Javascript concern, is this (or will this) be impacted by Nashhorns depreciation (as of jdk11 I believe) and potential removal at some point? I see reference to Netscape JObject do maybe that circumvents some of that but does this need to eventually transition to graalvm or other means? Eric Bresie ebre...@gmail.com > On March 10, 2020 at 6:22:01 AM CDT, Ajit Ghaisas > wrote: > On Fri, 6 Mar 2020 19:53:04 GMT, Kevin Rushforth wrote: > > > > Please ignore this comment as well, it is also for debugging issues with > > > Skara and mailman :email: > > > > @aghaisas can you review this? > > I executed these tests without this patch. They did not hang on my Macbook. > > I applied patch and executed the tests. They continue to pass. > I see the merit of this change to make tests predictable. +1. > > - > > PR: https://git.openjdk.java.net/jfx/pull/134
Re: [Integrated] RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
Changeset: 50e15fce Author:Kevin Rushforth Date: 2020-03-10 12:42:02 + URL: https://git.openjdk.java.net/jfx/commit/50e15fce 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang Reviewed-by: aghaisas ! tests/system/src/testapp5/java/mymod/myapp5/AppJSCallbackExported.java ! tests/system/src/testapp5/java/mymod/myapp5/AppJSCallbackOpened.java ! tests/system/src/testapp5/java/mymod/myapp5/AppJSCallbackQualExported.java ! tests/system/src/testapp5/java/mymod/myapp5/AppJSCallbackQualOpened.java ! tests/system/src/testapp5/java/mymod/myapp5/AppJSCallbackUnexported.java
Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
On Fri, 6 Mar 2020 19:53:04 GMT, Kevin Rushforth wrote: >> Please ignore this comment as well, it is also for debugging issues with >> Skara and mailman :email: > > @aghaisas can you review this? I executed these tests without this patch. They did not hang on my Macbook. I applied patch and executed the tests. They continue to pass. I see the merit of this change to make tests predictable. +1. - PR: https://git.openjdk.java.net/jfx/pull/134
Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
On Tue, 3 Mar 2020 22:51:43 GMT, Kevin Rushforth wrote:
> While testing JavaFX using gradle 6.3-nightly and JDK 14, I ran into what
> turned out to be a latent test bug in the
> AppJSCallback* apps that are launched by ModuleLauncherTest. The launched
> apps construct a WebView instance, obtain a
> WebEngine instance from the WebView, add a state listener to the WebEngine,
> load the web content, and then return from
> the Application::start method. The WebView instance is held a local variable,
> and so is subject to garbage collection
> once it goes out of scope when the start method returns. In addition, the
> test apps did not check for successful
> launching of the application or successful loading of the web content, which
> led to the test suite hanging (else it
> would have been a simple test failure). Note that I don't know (nor care)
> what changed in JDK 14 to make this more
> likely to occur, but since the test itself is demonstrably wrong, it needs to
> be fixed. The main part of the fix was
> to make the WebEngine an instance variable. In order to make the test more
> robust, I also modified it to launch the
> application in another thread, and having the main thread check that the
> application launched successfully and that the
> web content was loaded successfully, after which I did the assertion check
> for the correct number of callbacks. The 5
> different apps only differ from each other in the name of the class, the name
> of the package from which MyCallback is
> imported, and possibly the expected number of callbacks. I diffed
> AppJSCallbackExported.java against the other 4 test
> files and the diffs are essentially the same as they were before this change.
> Reviewers thus only need to review one of
> the tests in detail. For example, here are the diffs between
> AppJSCallbackExported and AppJSCallbackUnexported: $
> diff .../AppJSCallbackExported.java .../AppJSCallbackUnexported.java 37c37 <
> import myapp5.pkg2.MyCallback;
> ---
>> import myapp5.pkg1.MyCallback;
> 45c45
> < public class AppJSCallbackExported extends Application {
> ---
>> public class AppJSCallbackUnexported extends Application {
> 77c77
> < Util.assertEquals(1, callbackCount);
> ---
>> Util.assertEquals(0, callbackCount);
Marked as reviewed by aghaisas (Reviewer).
-
PR: https://git.openjdk.java.net/jfx/pull/134
Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
On Wed, 4 Mar 2020 14:11:42 GMT, Erik Helin wrote: >> Please ignore this comment, this is for debugging Skara that might have some >> mailing list mirroring issues > > Please ignore this comment as well, it is also for debugging issues with > Skara and mailman :email: @aghaisas can you review this? - PR: https://git.openjdk.java.net/jfx/pull/134
Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
On Wed, 4 Mar 2020 14:02:51 GMT, Erik Helin wrote:
>> While testing JavaFX using gradle 6.3-nightly and JDK 14, I ran into what
>> turned out to be a latent test bug in the AppJSCallback* apps that are
>> launched by ModuleLauncherTest. The launched apps construct a WebView
>> instance, obtain a WebEngine instance from the WebView, add a state listener
>> to the WebEngine, load the web content, and then return from the
>> Application::start method. The WebView instance is held a local variable,
>> and so is subject to garbage collection once it goes out of scope when the
>> start method returns.
>>
>> In addition, the test apps did not check for successful launching of the
>> application or successful loading of the web content, which led to the test
>> suite hanging (else it would have been a simple test failure).
>>
>> Note that I don't know (nor care) what changed in JDK 14 to make this more
>> likely to occur, but since the test itself is demonstrably wrong, it needs
>> to be fixed.
>>
>> The main part of the fix was to make the WebEngine an instance variable. In
>> order to make the test more robust, I also modified it to launch the
>> application in another thread, and having the main thread check that the
>> application launched successfully and that the web content was loaded
>> successfully, after which I did the assertion check for the correct number
>> of callbacks.
>>
>> The 5 different apps only differ from each other in the name of the class,
>> the name of the package from which MyCallback is imported, and possibly the
>> expected number of callbacks. I diffed AppJSCallbackExported.java against
>> the other 4 test files and the diffs are essentially the same as they were
>> before this change. Reviewers thus only need to review one of the tests in
>> detail.
>>
>> For example, here are the diffs between AppJSCallbackExported and
>> AppJSCallbackUnexported:
>>
>> $ diff .../AppJSCallbackExported.java .../AppJSCallbackUnexported.java
>>
>> 37c37
>> < import myapp5.pkg2.MyCallback;
>> ---
>>> import myapp5.pkg1.MyCallback;
>> 45c45
>> < public class AppJSCallbackExported extends Application {
>> ---
>>> public class AppJSCallbackUnexported extends Application {
>> 77c77
>> < Util.assertEquals(1, callbackCount);
>> ---
>>> Util.assertEquals(0, callbackCount);
>
> Please ignore this comment, this is for debugging Skara that might have some
> mailing list mirroring issues
Please ignore this comment as well, it is also for debugging issues with Skara
and mailman :email:
-
PR: https://git.openjdk.java.net/jfx/pull/134
Re: RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
On Tue, 3 Mar 2020 22:51:43 GMT, Kevin Rushforth wrote:
> While testing JavaFX using gradle 6.3-nightly and JDK 14, I ran into what
> turned out to be a latent test bug in the AppJSCallback* apps that are
> launched by ModuleLauncherTest. The launched apps construct a WebView
> instance, obtain a WebEngine instance from the WebView, add a state listener
> to the WebEngine, load the web content, and then return from the
> Application::start method. The WebView instance is held a local variable, and
> so is subject to garbage collection once it goes out of scope when the start
> method returns.
>
> In addition, the test apps did not check for successful launching of the
> application or successful loading of the web content, which led to the test
> suite hanging (else it would have been a simple test failure).
>
> Note that I don't know (nor care) what changed in JDK 14 to make this more
> likely to occur, but since the test itself is demonstrably wrong, it needs to
> be fixed.
>
> The main part of the fix was to make the WebEngine an instance variable. In
> order to make the test more robust, I also modified it to launch the
> application in another thread, and having the main thread check that the
> application launched successfully and that the web content was loaded
> successfully, after which I did the assertion check for the correct number of
> callbacks.
>
> The 5 different apps only differ from each other in the name of the class,
> the name of the package from which MyCallback is imported, and possibly the
> expected number of callbacks. I diffed AppJSCallbackExported.java against the
> other 4 test files and the diffs are essentially the same as they were before
> this change. Reviewers thus only need to review one of the tests in detail.
>
> For example, here are the diffs between AppJSCallbackExported and
> AppJSCallbackUnexported:
>
> $ diff .../AppJSCallbackExported.java .../AppJSCallbackUnexported.java
>
> 37c37
> < import myapp5.pkg2.MyCallback;
> ---
>> import myapp5.pkg1.MyCallback;
> 45c45
> < public class AppJSCallbackExported extends Application {
> ---
>> public class AppJSCallbackUnexported extends Application {
> 77c77
> < Util.assertEquals(1, callbackCount);
> ---
>> Util.assertEquals(0, callbackCount);
Please ignore this comment, this is for debugging Skara that might have some
mailing list mirroring issues
-
PR: https://git.openjdk.java.net/jfx/pull/134
RFR: 8240466: AppJSCallback* apps launched by ModuleLauncherTest intermittently hang
While testing JavaFX using gradle 6.3-nightly and JDK 14, I ran into what
turned out to be a latent test bug in the AppJSCallback* apps that are launched
by ModuleLauncherTest. The launched apps construct a WebView instance, obtain a
WebEngine instance from the WebView, add a state listener to the WebEngine,
load the web content, and then return from the Application::start method. The
WebView instance is held a local variable, and so is subject to garbage
collection once it goes out of scope when the start method returns.
In addition, the test apps did not check for successful launching of the
application or successful loading of the web content, which led to the test
suite hanging (else it would have been a simple test failure).
Note that I don't know (nor care) what changed in JDK 14 to make this more
likely to occur, but since the test itself is demonstrably wrong, it needs to
be fixed.
The main part of the fix was to make the WebEngine an instance variable. In
order to make the test more robust, I also modified it to launch the
application in another thread, and having the main thread check that the
application launched successfully and that the web content was loaded
successfully, after which I did the assertion check for the correct number of
callbacks.
The 5 different apps only differ from each other in the name of the class, the
name of the package from which MyCallback is imported, and possibly the
expected number of callbacks. I diffed AppJSCallbackExported.java against the
other 4 test files and the diffs are essentially the same as they were before
this change. Reviewers thus only need to review one of the tests in detail.
For example, here are the diffs between AppJSCallbackExported and
AppJSCallbackUnexported:
$ diff .../AppJSCallbackExported.java .../AppJSCallbackUnexported.java
37c37
< import myapp5.pkg2.MyCallback;
---
> import myapp5.pkg1.MyCallback;
45c45
< public class AppJSCallbackExported extends Application {
---
> public class AppJSCallbackUnexported extends Application {
77c77
< Util.assertEquals(1, callbackCount);
---
> Util.assertEquals(0, callbackCount);
-
Commits:
- c70d3995: 8240466: AppJSCallback* apps launched by ModuleLauncherTest
intermittently hang
Changes: https://git.openjdk.java.net/jfx/pull/134/files
Webrev: https://webrevs.openjdk.java.net/jfx/134/webrev.00
Issue: https://bugs.openjdk.java.net/browse/JDK-8240466
Stats: 265 lines in 5 files changed: 205 ins; 5 del; 55 mod
Patch: https://git.openjdk.java.net/jfx/pull/134.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/134/head:pull/134
PR: https://git.openjdk.java.net/jfx/pull/134
