Skip to content

8346952 : GetGraphicsStressTest.java fails: Native resources unavailable #25619

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

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 11 additions & 21 deletions src/java.desktop/windows/native/libawt/windows/awt_Window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2498,32 +2498,22 @@ jint AwtWindow::_GetScreenImOn(void *param)

jobject self = (jobject)param;

// It's entirely possible that our native resources have been destroyed
// before our java peer - if we're dispose()d, for instance.
// Alert caller w/ IllegalComponentStateException.
if (self == NULL) {
JNU_ThrowByName(env, "java/awt/IllegalComponentStateException",
"Peer null in JNI");
return 0;
}
PDATA pData = JNI_GET_PDATA(self);
if (pData == NULL) {
JNU_ThrowByName(env, "java/awt/IllegalComponentStateException",
"Native resources unavailable");
env->DeleteGlobalRef(self);
return 0;
}
jint result = -1;
AwtWindow* window = NULL;

jint result = 0;
AwtWindow *w = (AwtWindow *)pData;
if (::IsWindow(w->GetHWnd()))
// Our native resources may have been destroyed before the Java peer,
// e.g., if dispose() was called. In that case, return the default screen.
PDATA pData;
JNI_CHECK_PEER_GOTO(self, ret);
Copy link
Member

@mrserb mrserb Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest reordering it slightly - this pattern is commonly used in most cases where JNI_CHECK_PEER_GOTO is used:

    result...;
    AwtWindow *window = NULL;

    PDATA pData;
    JNI_CHECK_PEER_GOTO(self, ret);
    window = (AwtWindow *)pData;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still suggest this order.

Also, please add the new bug ID to the GetGraphicsStressTest.
btw I'm still looking into the new test - it always passes for me. Can we tweak it to reproduce the bug more reliably?

Copy link
Contributor Author

@anass-baya anass-baya Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cant reproduce it locally.
I can see it only on CI 20/20 with the new test and 2/20 with the old one

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the stack trace when the new test fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same stack as the old test. However let me try to enhance it and come back to you

Copy link
Contributor Author

@anass-baya anass-baya Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @mrserb,
I've enhanced the new test. It now reproduces the issue consistently (5/5) locally, but only intermittently (6/20) on CI.
Can we proceed like that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @mrserb,
What do you think about the test? Does it look good to you?
KR,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking into it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unclear from the diff, but does the PDATA pData have additional space at the beginning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. it's handled now

window = (AwtWindow *)pData;
if (::IsWindow(window->GetHWnd()))
{
result = (jint)w->GetScreenImOn();
result = (jint)window->GetScreenImOn();
}

ret:
env->DeleteGlobalRef(self);

return result;
return (result != -1) ? result : AwtWin32GraphicsDevice::GetDefaultDeviceIndex();
}

void AwtWindow::_SetFocusableWindow(void *param)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

import java.awt.Window;

/**
* @test
* @bug 8346952
* @summary Verifies no exception occurs when triggering updateCG()
* for an ownerless window.
* @key headful
*/
public final class BogusFocusableWindowState {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please split the long line to 80 chars, also the empty line above is not really necessary.

Copy link
Contributor Author

@anass-baya anass-baya Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello @mrserb,
Of course, it's done. Thank you.
For my proposed test, it does trigger the same exception call stack.
However, yours is better. The reproduction frequency with it is 20/20.


public static void main(String[] args) {
Window frame = new Window(null) {
@Override
public boolean getFocusableWindowState() {
removeNotify();
return true;
}
};
try {
frame.pack();
frame.setVisible(true);
} finally {
frame.dispose();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

/**
* @test
* @bug 8235638 8235739 8285094
* @bug 8235638 8235739 8285094 8346952
* @key headful
*/
public final class GetGraphicsStressTest {
Expand Down