[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] What is the point of CloseTTFontFileFunc?
From:       Andrew Dinn <adinn () redhat ! com>
Date:       2015-01-16 9:19:33
Message-ID: 54B8D7A5.9030903 () redhat ! com
[Download RAW message or body]

On 15/01/15 22:53, Phil Race wrote:
> I can only suppose the author thought he was going to pass the
> platName (which would be the font file path) to some code that would
> use it to somehow close an open descriptor on that file. But clearly
> it does nothing of the sort and an up-call to the java font object
> (like in the read function) would not need that information anyway.
> So it looks redundant. I don't see how it would relate to the crash.
> That *may* be caused by some simultaneous use and/or freeing in
> different threads of files created using Font.createFont(..) and are=20
> now bing released. I've also seen crashes where admins clean out the
> /tmp folder whilst the JRE is running using font files there. But
> that would probably not involve the disposer code.

The crash has been pinned down to some faulty argument passing. However,
I am glad to hear that the code appears to be redundant. My concern was
that it might be worse than that since the native code calls
JNU_ReleaseStringPlatformChars which, potentially***, can release
memory. Since platName is created by Java code (Using a combination of
String literals and String+ operations) that would be a /very bad/
thing. So, I think this callback should probably be removed (as an a
fortiori argument I'll note that it is, at best, redundant).

***n.b. I say potentially because the implementation of
JNU_ReleaseStringPlatformChars is platform-specific and has many
different code paths depending upon the type of the text the String
holds. So, I am not sure the callback will ever free memory on any
platform (Windows looks to be the most likely candidate).

regards,


Andrew Dinn
-----------
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic