[11] Review request: 8087581: Headless Monocle causes core dump when rendering javafx.scene.text.Text

[Alex Kashchenko]

Hi OpenJFX developers,

issue: https://bugs.openjdk.java.net/browse/JDK-8087581
webrev: http://cr.openjdk.java.net/~akasko/ojfx/8087581/webrev.01/

I am working on windows builds at Red Hat, and some time ago stumbled 
upon the windows-only font-related crash inside directwrite.cpp. Crash 
was 100% reproducible (but only with openjdk8+openjfx - not with Oracle 
jdk) with VocabHunter [1] project - running:


gradlew :gui:test

Problem appeared to be the same as logged in JDK-8087581. It was 
narrowed down to this call [2].


Creating COM object instance and immediately un-initializing COM seemed 
to be a wrong approach. Looking how COM is used in similar manner 
(singleton instance accessed from a single thread) in other projects 
[3][4], it appeared that there CoUninitialize either called on thread 
exit or not called at all.


In the proposed patch CoUninitialize is tied to this cleanup call [5].


Please note some possible problems with this patch, that may prevent it 
from being accepted:


 - patch is tested only with openjdk-8u161 + openjfx-8u161
 - I was unable to reproduce the problem with Oracle jdk
 - reproducer inside JDK-8087581 seems to be broken with recent jdk8 
(something is wrong with headless monocle lib [6] ), I used only 
VocabHunter project as a reproducer
 - the latest openjfx build I have is from openjfx/10/rt/ repo - do not 
have 11 (or dev) builds yet
 - VocabHunter cannot be built with openjfx-10, so the gradle command 
above won't work with it
 - I bundled reproducer as a standalone JAR, but it is still not a real 
reproducer, as that JAR is 60MB (with guice and other libs inside) and 
won't run on openjfx-10 (again some headless monocle incompatibility)
 - fontFactory is changed to "protected" here [7], to call the 
dispose() on it directly from D3DPipeline; it may be better to call 
dispose() for any fontFactory from GraphicsPipeline (parent) , but in 
testing it appeared to be, that parent's dispose() is never called from 
D3DPipeline.dispose(), as nDispose() [8] call inside it never exits 
(process exits first); thus in patch fontFactory.dispose() is called 
before the nDispose() call



[1] https://github.com/VocabHunter/VocabHunter
[2] http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/86a1b36090e4#l22.599
[3] https://hg.mozilla.org/mozilla-central/rev/a67425aa4728#l1.44
[4] 
https://github.com/Microsoft/Windows-driver-samples/blob/master/print/XpsRasFilter/src/BitmapHandler.cpp#L64
[5] 
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/file/375aedc5702a/modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/QuantumRenderer.java#l127

[6] http://mvnrepository.com/artifact/org.testfx/openjfx-monocle
[7] 
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/file/375aedc5702a/modules/javafx.graphics/src/main/java/com/sun/prism/GraphicsPipeline.java#l58
[8] 
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/file/375aedc5702a/modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DPipeline.java#l164



--
-Alex

Re: Update on enabling JavaFX to work with OpenJDK builds

[Kevin Rushforth]

This seems reasonable to me as well: integrating the JavaFX modules into 
whatever dependency manager you are using seems like a good thing.


-- Kevin


Johan Vos wrote:

Hi Michael,

The second question is what I think should be the default.
This is similar to Java EE ^^^ EE4J  Jakarta EE development: if you
want to include (some) EE API's, you include them in maven/gradle/ant, or
your IDE does that for you.

Same, if you want to include JavaFX API's in your project, you declare that
as a dependency in maven/gradle/ant (or your IDE does that for you) and
they will get downloaded from Maven Central or jcenter (if you don't have
them yet).

- Johan

Op wo 28 feb. 2018 om 10:29 schreef Michael Paus :

  

Hi Kevin,
thank you for the update. I appreciate this work very much because I
think it is utterly important for JavaFX.
While reading the past messages again I was wondering whether this will
also address the following
two questions.
1. Will there be a way to determine at program start-up whether JavaFX
is available or not in order to be able
to give a user a clear and helpful error message in case it is not?
2. Might it even be possible to treat JavaFX as just another dependency
in your Maven (or whatever) project?
For me personally these questions are not so important because I prefer
the approach of building a native
installer which already contains everything needed, but for other people
in other usage scenarios these
questions might be important.
--Michael

Am 27.02.18 um 22:21 schrieb Kevin Rushforth:


One of the big challenges in running JavaFX with OpenJDK builds is
that developers need to build OpenJDK locally themselves and include
the JavaFX bits produced by a locally-built OpenJFX build.

In an earlier email with the subject "javafx might not be present"
[1], I mentioned our intention to make it easier for OpenJFX to be
built, tested, and run with OpenJDK builds that don't already contain
javafx.* modules. This will pave the way to allow a set of pre-built
javafx modules to be distributed that will run on top of a pre-built
OpenJDK.

By way of update, this work is underway and can be tracked via the
following two JBS issues:

1. Removing internal dependencies:

JDK-8195798 [2] : Address dependencies in javafx.* modules on internal
APIs of core modules

The above is an umbrella task that points to several linked blocking
bugs. All of these bugs are in progress and a few are already done
(e.g., removing jdk.internal.Unsafe from Marlin).


2. Enabling the building, testing, and distribution as a set of
separate modules

JDK-8198329 [3] : Support FX build / test using JDK that doesn't
include javafx.* modules

This one depends on the first two, but can be started in parallel, so
I plan to start on it in the next few days.


Let me know if you have any questions on this.

-- Kevin

[1]

  

http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-February/021447.html


[2] https://bugs.openjdk.java.net/browse/JDK-8195798

[3] https://bugs.openjdk.java.net/browse/JDK-8198329

  

Re: Update on enabling JavaFX to work with OpenJDK builds

[Michael Paus]

I think this feature would be nice to have but I doubt that it is simple 
to achieve
because JavaFX may or may not already be included in the JDK against 
which you build
and on which your project later runs. There needs to be a strategy how 
to handle all

the resulting corner cases.

Am 28.02.18 um 11:27 schrieb Johan Vos:

Hi Michael,

The second question is what I think should be the default.
This is similar to Java EE ^^^ EE4J  Jakarta EE development: 
if you want to include (some) EE API's, you include them in 
maven/gradle/ant, or your IDE does that for you.


Same, if you want to include JavaFX API's in your project, you declare 
that as a dependency in maven/gradle/ant (or your IDE does that for 
you) and they will get downloaded from Maven Central or jcenter (if 
you don't have them yet).


- Johan

Op wo 28 feb. 2018 om 10:29 schreef Michael Paus >:


Hi Kevin,
thank you for the update. I appreciate this work very much because I
think it is utterly important for JavaFX.
While reading the past messages again I was wondering whether this
will
also address the following
two questions.
1. Will there be a way to determine at program start-up whether JavaFX
is available or not in order to be able
to give a user a clear and helpful error message in case it is not?
2. Might it even be possible to treat JavaFX as just another
dependency
in your Maven (or whatever) project?
For me personally these questions are not so important because I
prefer
the approach of building a native
installer which already contains everything needed, but for other
people
in other usage scenarios these
questions might be important.
--Michael

Am 27.02.18 um 22:21 schrieb Kevin Rushforth:
> One of the big challenges in running JavaFX with OpenJDK builds is
> that developers need to build OpenJDK locally themselves and include
> the JavaFX bits produced by a locally-built OpenJFX build.
>
> In an earlier email with the subject "javafx might not be present"
> [1], I mentioned our intention to make it easier for OpenJFX to be
> built, tested, and run with OpenJDK builds that don't already
contain
> javafx.* modules. This will pave the way to allow a set of pre-built
> javafx modules to be distributed that will run on top of a pre-built
> OpenJDK.
>
> By way of update, this work is underway and can be tracked via the
> following two JBS issues:
>
> 1. Removing internal dependencies:
>
> JDK-8195798 [2] : Address dependencies in javafx.* modules on
internal
> APIs of core modules
>
> The above is an umbrella task that points to several linked blocking
> bugs. All of these bugs are in progress and a few are already done
> (e.g., removing jdk.internal.Unsafe from Marlin).
>
>
> 2. Enabling the building, testing, and distribution as a set of
> separate modules
>
> JDK-8198329 [3] : Support FX build / test using JDK that doesn't
> include javafx.* modules
>
> This one depends on the first two, but can be started in
parallel, so
> I plan to start on it in the next few days.
>
>
> Let me know if you have any questions on this.
>
> -- Kevin
>
> [1]
>
http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-February/021447.html
>
> [2] https://bugs.openjdk.java.net/browse/JDK-8195798
>
> [3] https://bugs.openjdk.java.net/browse/JDK-8198329
>

Re: Repositories, AdoptOpenJDK and github

[Johan Vos]

I agree with this approach.
Having a small number of JBS bugs that are low hanging fruit will be good
to see how the flow works.
Eugene created an easy one: https://bugs.openjdk.java.net/browse/JDK-8198795

- Johan

On Wed, Feb 28, 2018 at 9:52 AM Laurent Bourgès 
wrote:

> Johan,
> I am following the long discussion and I mostly agree what was said.
>
> Maybe it is time to start working on github on few minor / trivial bugs...
> to test all the new process.
>
> I propose to extract few JBS bugs (small) with high ROI (agile /scrum
> approach) and create shadow copies into github issues (id, title, short
> description & JBS LINK) to be proposed to the jfx community.
>
> That would become our backlog that can be managed in a kanban board
> (github project). Moreover it would be awesome if such board would gather
> the activity of both oracle & community people on OpenJFX.
>
> Once somebody wants to work on one issue, just comment on the github issue
> and start working in your fork, make PR...
>
> Adopting this method, anybody will know publicly what's going on and it
> would reduce the risk of conflicts (code merge)
>
> My 2 cents...
>
> Let's go on,
> "We are the champions..."
>
> Laurent
>
>
> Le 28 févr. 2018 9:15 AM, "Johan Vos"  a écrit :
>
> That is the difficult point indeed.
> But why would a PR to OpenJFX be rejected after it was approved in the
> github mirror? I would assume the main reason for this is because the PR
> did not what it was supposed to do. In that case, it makes sense to remove
> the commits from the github mirror as well.
>
> I think the main thing here is that we need to be very serious about
> reviewing and accepting PR's in the github mirror. Accepting a PR in github
> does not require the *formal* process of creating webrevs etc, but it
> requires discussion about the issue with reviewers of OpenJFX.
> We have to minimize the number of times an edge case occurs, in which the
> discussion was pro PR first, but after it got merged into "development" new
> arguments are brought up against the PR.
> I think it would be good to have sort of a post-mortem analysis in case
> this happens, in order to prevent it from happening again. But as I said,
> if it does happen, it probably has good reasons and in that case we have to
> remove it from the development branch as well.
>
> I think the more common case would be that an issue is fixed on the github
> mirror, but not yet accepted (nor rejected) in OpenJFX, so there will be
> some time lag between the PR acceptance and the integration in OpenJFX. But
> this should not be a problem, as long as the following scenario is the main
> flow:
>
> The github master branch is always synced with OpenJFX, and never gets
> modified by other commits.
> The github "development" branch is where we accept PR's, that can then be
> send upstream. Changes from "master" are regularly merged into
> "development". The moment an accepted PR makes it into OpenJFX, it will be
> synced into "master" and merged into "development" where the merge happens
> silently as there are no conflicts (since development already has this
> code).
>
> Does that make sense?
>
> - Johan
>
> On Wed, Feb 28, 2018 at 12:51 AM Kevin Rushforth <
> kevin.rushfo...@oracle.com>
> wrote:
>
> >
> >
> > Nir Lisker wrote:
> >
> > Johan's thinking was to allow Committers to approve the PR on GitHub --
> >> meaning they could be merged on GitHub before an actual Review has
> >> happened. Are you proposing to change that?
> >
> >
> > What if the PR is rejected at review? We'll end up with conflicts between
> > the repos. And supposed someone works on a different fix and uses the
> > rejected PR code, how will that be committed?
> >
> >
> > Good questions; maybe Johan has some thoughts as to how to mitigate this?
> >
> >
> > -- Kevin
> >
> >
> >
> > On Wed, Feb 28, 2018 at 12:25 AM, Kevin Rushforth <
> > kevin.rushfo...@oracle.com> wrote:
> >
> >> This seems a good start in formalizing the process. It will need a
> little
> >> tweaking in a couple of areas.
> >>
> >> Regarding JBS access, even though I want to relax the requirement to
> >> become an Author (get a JBS account), it will likely end up somewhere
> >> between "an intention to contribute" and "two sponsored contributions,
> >> already reviewed and committed". Even without this, there will
> necessarily
> >> be a gap in time between "I want to work on a bug" and getting a JBS
> >> account. So there is value in encouraging people to clone the GitHub
> >> sandbox, "kick the tires", make a PR to get feedback, etc., before they
> can
> >> access JBS directly (or even while waiting for their OCA to be
> processed,
> >> but no PRs in that case). Something to take into account.
> >>
> >> Regarding review, we will need a bit more discussion on that. I like the
> >> idea of the PR being logged in JBS once it is ready to be reviewed.
> Johan's
> >> thinking was to allow Committers to 

Re: Update on enabling JavaFX to work with OpenJDK builds

[Michael Paus]

Hi Kevin,
thank you for the update. I appreciate this work very much because I 
think it is utterly important for JavaFX.
While reading the past messages again I was wondering whether this will 
also address the following

two questions.
1. Will there be a way to determine at program start-up whether JavaFX 
is available or not in order to be able

to give a user a clear and helpful error message in case it is not?
2. Might it even be possible to treat JavaFX as just another dependency 
in your Maven (or whatever) project?
For me personally these questions are not so important because I prefer 
the approach of building a native
installer which already contains everything needed, but for other people 
in other usage scenarios these

questions might be important.
--Michael

Am 27.02.18 um 22:21 schrieb Kevin Rushforth:
One of the big challenges in running JavaFX with OpenJDK builds is 
that developers need to build OpenJDK locally themselves and include 
the JavaFX bits produced by a locally-built OpenJFX build.


In an earlier email with the subject "javafx might not be present" 
[1], I mentioned our intention to make it easier for OpenJFX to be 
built, tested, and run with OpenJDK builds that don't already contain 
javafx.* modules. This will pave the way to allow a set of pre-built 
javafx modules to be distributed that will run on top of a pre-built 
OpenJDK.


By way of update, this work is underway and can be tracked via the 
following two JBS issues:


1. Removing internal dependencies:

JDK-8195798 [2] : Address dependencies in javafx.* modules on internal 
APIs of core modules


The above is an umbrella task that points to several linked blocking 
bugs. All of these bugs are in progress and a few are already done 
(e.g., removing jdk.internal.Unsafe from Marlin).



2. Enabling the building, testing, and distribution as a set of 
separate modules


JDK-8198329 [3] : Support FX build / test using JDK that doesn't 
include javafx.* modules


This one depends on the first two, but can be started in parallel, so 
I plan to start on it in the next few days.



Let me know if you have any questions on this.

-- Kevin

[1] 
http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-February/021447.html


[2] https://bugs.openjdk.java.net/browse/JDK-8195798

[3] https://bugs.openjdk.java.net/browse/JDK-8198329

Re: Repositories, AdoptOpenJDK and github

[Laurent Bourgès]

Johan,
I am following the long discussion and I mostly agree what was said.

Maybe it is time to start working on github on few minor / trivial bugs...
to test all the new process.

I propose to extract few JBS bugs (small) with high ROI (agile /scrum
approach) and create shadow copies into github issues (id, title, short
description & JBS LINK) to be proposed to the jfx community.

That would become our backlog that can be managed in a kanban board (github
project). Moreover it would be awesome if such board would gather the
activity of both oracle & community people on OpenJFX.

Once somebody wants to work on one issue, just comment on the github issue
and start working in your fork, make PR...

Adopting this method, anybody will know publicly what's going on and it
would reduce the risk of conflicts (code merge)

My 2 cents...

Let's go on,
"We are the champions..."

Laurent


Le 28 févr. 2018 9:15 AM, "Johan Vos"  a écrit :

That is the difficult point indeed.
But why would a PR to OpenJFX be rejected after it was approved in the
github mirror? I would assume the main reason for this is because the PR
did not what it was supposed to do. In that case, it makes sense to remove
the commits from the github mirror as well.

I think the main thing here is that we need to be very serious about
reviewing and accepting PR's in the github mirror. Accepting a PR in github
does not require the *formal* process of creating webrevs etc, but it
requires discussion about the issue with reviewers of OpenJFX.
We have to minimize the number of times an edge case occurs, in which the
discussion was pro PR first, but after it got merged into "development" new
arguments are brought up against the PR.
I think it would be good to have sort of a post-mortem analysis in case
this happens, in order to prevent it from happening again. But as I said,
if it does happen, it probably has good reasons and in that case we have to
remove it from the development branch as well.

I think the more common case would be that an issue is fixed on the github
mirror, but not yet accepted (nor rejected) in OpenJFX, so there will be
some time lag between the PR acceptance and the integration in OpenJFX. But
this should not be a problem, as long as the following scenario is the main
flow:

The github master branch is always synced with OpenJFX, and never gets
modified by other commits.
The github "development" branch is where we accept PR's, that can then be
send upstream. Changes from "master" are regularly merged into
"development". The moment an accepted PR makes it into OpenJFX, it will be
synced into "master" and merged into "development" where the merge happens
silently as there are no conflicts (since development already has this
code).

Does that make sense?

- Johan

On Wed, Feb 28, 2018 at 12:51 AM Kevin Rushforth 
wrote:

>
>
> Nir Lisker wrote:
>
> Johan's thinking was to allow Committers to approve the PR on GitHub --
>> meaning they could be merged on GitHub before an actual Review has
>> happened. Are you proposing to change that?
>
>
> What if the PR is rejected at review? We'll end up with conflicts between
> the repos. And supposed someone works on a different fix and uses the
> rejected PR code, how will that be committed?
>
>
> Good questions; maybe Johan has some thoughts as to how to mitigate this?
>
>
> -- Kevin
>
>
>
> On Wed, Feb 28, 2018 at 12:25 AM, Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
>> This seems a good start in formalizing the process. It will need a little
>> tweaking in a couple of areas.
>>
>> Regarding JBS access, even though I want to relax the requirement to
>> become an Author (get a JBS account), it will likely end up somewhere
>> between "an intention to contribute" and "two sponsored contributions,
>> already reviewed and committed". Even without this, there will
necessarily
>> be a gap in time between "I want to work on a bug" and getting a JBS
>> account. So there is value in encouraging people to clone the GitHub
>> sandbox, "kick the tires", make a PR to get feedback, etc., before they
can
>> access JBS directly (or even while waiting for their OCA to be processed,
>> but no PRs in that case). Something to take into account.
>>
>> Regarding review, we will need a bit more discussion on that. I like the
>> idea of the PR being logged in JBS once it is ready to be reviewed.
Johan's
>> thinking was to allow Committers to approve the PR on GitHub -- meaning
>> they could be merged on GitHub before an actual Review has happened. Are
>> you proposing to change that? It might have some advantages, but it could
>> also make it harder in other areas. I'd like to hear from Johan on this.
>> This reminds me that we need to continue the discussion on the general
>> "Review" policy, as it is relevant here.
>>
>> As for whether it is merged into GitHub, I don't have a strong opinion on
>> that. As you say it will be pulled into the mirror 

Re: Update on enabling JavaFX to work with OpenJDK builds

[Paul Ray Russell]

 "I mentioned our intention to make it easier for OpenJFX to be built,
tested, and run with OpenJDK builds that don't already contain javafx.*
modules."

+1 Great