Replace the static error string in ZIP_Put_In_Cache0 with on stack memory
coleen.phillimore at oracle.com
Wed Apr 11 12:01:08 PDT 2012
I don't actually like this change at all. If you use hotspot with an
old version of the JDK without this char* being allocated in C heap,
we'll end up with an extra free which would be a memory stomp or crash.
This change doesn't motivate losing backward compatibility and this
sort of crash is hard to debug.
It doesn't look like we do anything with the error_msg, just return a
static string "an error occurred" or something instead and free it in
On 4/11/2012 10:58 AM, Sean Chou wrote:
> Hi hotspot guys,
> Would any one like to take a look at this? I'm trying to fix a
> potential race in ZIP_Open, it is found classLoader.cpp uses this
> function. So a webrev for hotspot is made as well, but I need a
> sponsor from hotspot as suggested by Alan Bateman.
> The start of this thread is
> The webrevs:
> and http://cr.openjdk.java.net/~zhouyx/7159982/webrev.00/
> <http://cr.openjdk.java.net/%7Ezhouyx/7159982/webrev.00/> .
> And, I found JDK_Version::is_gte_jdk18x_version suggested by Alan
> does not exist in hotspot yet, shall I add it to the patch too ?
> On Tue, Apr 10, 2012 at 4:10 PM, Alan Bateman <Alan.Bateman at oracle.com
> <mailto:Alan.Bateman at oracle.com>> wrote:
> On 10/04/2012 08:10, Sean Chou wrote:
> And it is found that hotspot calls ZIP_Open through
> (*ZipOpen) in file classLoader.cpp .So I also made a patch for
> it and add hotspot-dev to cc list.
> File classLoader.cpp is the only one I have found calling
> The webrev for hotspot is:
> To hotspot guys,
> We are trying to make the error path in ZIP_Open in
> src/share/native/java/util/zip/zip_util.c thread safe by
> changing the errbuf from static array to on stack array.
> This will cause the returned error string allocated from heap,
> which need to be freed. I checked the code and found only
> classLoader.cpp calls this function, so I made the above
> webrev. Please take a look.
> It's good that you spotted that this is called from the VM (I'd
> forgotten that). In that case things get a bit more complicated
> because the hotspot repository takes a different route into master
> and also goes into 7u releases too. I'll leave the HotSpot group
> to comment on your proposed change but I assume it will require
> adding JDK_Version::is_gte_jdk18x_version so that the JDK version
> can be tested before decided whether to free the error message. It
> will also require a sponsor as you cannot push changes to
> jdk8/tl/hotspot, all HotSpot changes require going through a
> build+test system (no direct pushes).
> Best Regards,
> Sean Chou
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-dev