Replace the static error string in ZIP_Put_In_Cache0 with on stack memory

[Coleen Phillimore]

Hi,
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 
the JDK.
thanks,
Coleen

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 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-April/009766.html 
> .
>     The webrevs: 
> http://cr.openjdk.java.net/~zhouyx/7159982/webrev-hotspot.00/ 
> <http://cr.openjdk.java.net/%7Ezhouyx/7159982/webrev-hotspot.00/>
> 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
>         ZIP_Open.
>
>         The webrev for hotspot is:
>         http://cr.openjdk.java.net/~zhouyx/7159982/webrev-hotspot.00/
>         <http://cr.openjdk.java.net/%7Ezhouyx/7159982/webrev-hotspot.00/>
>         <http://cr.openjdk.java.net/%7Ezhouyx/7159982/webrev-hotspot.00/>
>
>
>
>         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).
>
>     -Alan
>
>
>
>
> -- 
> Best Regards,
> Sean Chou
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20120411/ecd5b188/attachment-0001.html