RFR [8046339] sun.rmi.transport.DGCAckHandler leaks memory

[Stuart Marks]

Hi Ivan,

Sorry for not having gotten back to you earlier about this.

I'm uncomfortable with pushing in such a fix in the absence of a reproducer 
and/or a test case. Although the change itself seems sensible, the fact that we 
lack a reproducer means that we might not actually understand the problem. There 
is a lot of deep history in this area and it's very subtle, so I'd prefer not to 
proceed with fixes in the absence of a full understanding of the situation and a 
test case.

I don't think a test would actually have to run the system out of memory. It 
should be sufficient to use something like weak references or reflection to 
inspect the table to ensure that the reference of interest is no longer present 
after having executed the code path in question. The trick is to ensure that we 
exercise the timeout case instead of the normal release case. It's not obvious 
how to do that (and indeed that may have been the stumbling block in formulating 
the original test when this was first looked at a couple years ago).

Unfortunately I probably won't have time to dig into this until after JavaOne. 
We should pick it up after that, though.

s'marks



On 9/8/14 11:32 AM, Ivan Gerasimov wrote:
> This is a friendly reminder.
>
> Would anyone please be able to review this fix in RMI area?
> We have a few reports about this issue sometimes occurring, though no reliable
> reproducer.
> If someone comes up with the reproducer, the regtest can be added later.
>
> Sincerely yours,
> Ivan
>
> On 11.08.2014 17:41, Ivan Gerasimov wrote:
>> Hello everyone!
>>
>> It has been reported that under some conditions instances of
>> sun.rmi.transport.DGCAckHandler accumulate and can cause OOM Error.
>> This is because they are stored in the global DGCAckHandler.idTable map, and
>> may fail to be removed if a timeout has expired.
>>
>> The webrev contains a fix proposed by Darryl Mocek back in 2011.
>>
>> Unfortunately I couldn't come up with a regression test for the fix.
>> However, the fix looks obviously correct, especially taking into account the
>> comment to the constructor:
>>
>>      * References added to this DGCAckHandler will be held strongly
>>      * until its "release" method is invoked or (after the
>>      * "startTimer" method has been invoked) the timeout has expired.
>>
>> Would you please help review the fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8046339
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8046339/0/webrev/
>>
>> Sincerely yours,
>> Ivan
>>
>>
>>
>