Looks fine.

On 29/05/2019 11:38, Phil Race wrote:
Any takers ?

-phil.

On 5/24/19 9:35 AM, Phil Race wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8223271
Webrev : http://cr.openjdk.java.net/~prr/8223271/

Whilst working on removing some inappropriate coupling of the java launcher and 
the desktop module,
and testing out the -splash option, I noticed that on MacOS, in the case when 
we *default* to headless
mode (which happens when we determine that we are not in a desktop session), 
the launcher still
invokes the splash screen code. This could cause an application to completely 
hang.

The problem is explained in detail in the bug report, but briefly, the launcher 
isn't aware of this
defaulting to headless. And the calls the launcher makes to the dynamically 
loaded splashscreen
code, don't return any value it can use to be aware of this.

So this fix updates "DoSplashInit()" to return such a code, and in the event of 
a headful session
not being available, it can bail out and not try to show the splashscreen.
That is the gist of the small set of changes in java.c that relate to this fix.

However I also observed a small memory leak.
There are static variables

static char* splash_jar_entry = NULL; static char* splash_file_entry = NULL;
which are meant to hold the location of the splash image.
When everything is done the malloced storage these point to is freed ...
except that the malloc code looks like this :-

char* splash_file_entry = JLI_MemAlloc(JLI_StrLen(SPLASH_FILE_ENV_ENTRY 
"=")+JLI_StrLen(splash_file_name)+1);
char* splash_jar_entry = JLI_MemAlloc(JLI_StrLen(SPLASH_JAR_ENV_ENTRY 
"=")+JLI_StrLen(splash_jar_name)+1);

So the static vars are never used and the storage pointed to by local variables 
is never freed.

So I am fixing that too.

The rest of the fix is in the splashscreen code in the desktop module and it 
implements
the return of the status code described above.

There was one leak there too - a stream was not closed in a case where the 
splash could
not be displayed.

The regression tests for splash screen were run, but testing this is more an 
environmental issue -
sshing into a MacOS system and running tests and demos which would try to 
display a splash
and verifying they no longer do, so I didn't see a way to add a specific 
regression test.

-phil.








--
Best regards, Sergey.

Reply via email to