8341418: Prism/es2 DrawableInfo is never freed (leak)

[tsayao]

When creating a Scene, a DrawableInfo is allocated with malloc. When scene changes, this is called on WindowStage.java:

QuantumRenderer.getInstance().disposePresentable(painter.presentable); // latched on RT

But the underlying DrawableInfo is never freed.

I also think this should be done when the Stage is closed.

To test:

import javafx.animation.Animation;
import javafx.animation.KeyFrame;
import javafx.animation.KeyValue;
import javafx.animation.Timeline;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.TextField;
import javafx.scene.control.Label;
import javafx.scene.layout.Pane;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.stage.Stage;
import javafx.util.Duration;

public class TestScenes extends Application {

    @Override
    public void start(Stage stage) {
        Timeline timeline = new Timeline(
                new KeyFrame(Duration.millis(100), e -> stage.setScene(createScene("Scene 1", Color.RED))),
                new KeyFrame(Duration.millis(200), e -> stage.setScene(createScene("Scene 2", Color.BLUE))),
                new KeyFrame(Duration.millis(300), e -> stage.setScene(createScene("Scene 3", Color.GREEN)))
        );

        timeline.setCycleCount(Animation.INDEFINITE);
        timeline.play();

        stage.show();
    }

    private Scene createScene(String text, Color color) {
        return new Scene(new StackPane(), 400, 300, color);
    }

    public static void main(String[] args) {
        launch(TestScenes.class, args);
    }
}

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8341418: Prism/es2 DrawableInfo is never freed (leak) (Bug - P4)

Reviewers

• Michael Strauß (@mstr2 - Committer) 🔄 Re-review required (review applies to ad27a8d9)
• Lukasz Kostyra (@lukostyra - Committer) 🔄 Re-review required (review applies to ad27a8d9)
• Ambarish Rapte (@arapte - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1586/head:pull/1586
$ git checkout pull/1586

Update a local copy of the PR:
$ git checkout pull/1586
$ git pull https://git.openjdk.org/jfx.git pull/1586/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1586

View PR using the GUI difftool:
$ git pr show -t 1586

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1586.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back tsayao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[tsayao]

For now I'm looking for some guidance if this is the right path (In a broader sense, since I know I'm missing some details).

Also, I would like to confirm if I can open a formal JBS bug.

[tsayao]

On Ubuntu 20.04 with Xorg I get two failg test (most probably not related to the change):

[tsayao]

I have only tested on Linux. I don't own a Mac. I do have Windows, but it seems difficult to setup a build.
I'm unsure about Monocle EGL.

[mlbridge]

Webrevs

• 03: Full - Incremental (c0f38569)
• 02: Full - Incremental (ad27a8d9)
• 01: Full - Incremental (f5960383)
• 00: Full (c8a71d1e)

[kevinrushforth]

Reviewers: @arapte @lukostyra

/reviewers 2

[openjdk]

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[mstr2]

The fix works on Windows and macOS. I've left some additional comments.

[lukostyra]

LGTM, seems fine on Windows with ES2 backend

[arapte]

The fix itself looks good. Providing a few minor comments.

[arapte]

LGTM