RFR(S): 8013329: [parfait] File leak in hotspot/src/share/vm/compiler/compileBroker.cpp

[Albert Noll]

Hi Roland,

thanks again for looking at the code.

On 28.05.2013 15:21, Roland Westrelin wrote:
> Hi Albert,
>
>>> FREE_C_HEAP_ARRAY is for arrays allocated with NEW_C_HEAP_ARRAY. You should use delete here, I think (even though FREE_C_HEAP_ARRAY and delete end up doing the same).
>>>
>> The char array is allocated with NEW_HEAP_ARRAY in compileBroker.cpp. In the new version, I moved
>> the allocation of the array into the constructor of CompileLog. I think that this makes the code better readable. Do you agree?
> You're right. I misread the code.
> It's better if it's allocated from the constructor and freed from the destructor, indeed.
> Your change to CompileBroker::init_compiler_thread_log() is not entirely correct, I believe: if several temp dirs are tried, and the first ones fail, "Cannot open log file:" will be reported for the failed attempt even if the log file is successfully open in the end.
Thanks for pointing that out. The new version prints the warning only if 
no file can be opened.

>>> In CompileLog::finish_log_on_error, you delete log but use it afterwards log = log->_next.
>>>
>> Ouch :-( Thanks for this one. I think the current version should be fine.
> I think so as well.
>
> Roland.
When executing the current path with "fastdebug", the following warning 
appears:

Warning:  TraceDependencies results may be inflated by VerifyDependencies

I am not sure what that means. I tested with -XX:+LogCompilation and it 
seems to
work fine.

webrev: http://cr.openjdk.java.net/~anoll/8013329/webrev.02/ 
<http://cr.openjdk.java.net/%7Eanoll/8013329/webrev.02/>

Thanks again,
Albert
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20130531/13cdb61d/attachment.html