Hi Gary,
It looks good to me.
Nit:
It is better to define as interface:
+ private static RemoteHostImpl remoteHost;
=>
+ private static RemoteHost remoteHost;
+ private static RemoteVmImpl rvm;
=>
+ private static RemoteVm rvm;
Thanks,
Serguei
On 3/24/19 17:10, Gary Adams wrote:
Here's
an updated webrev that includes the change to bind to the stub.
The jstatd tests continue to pass after this change.
Webrev: http://cr.openjdk.java.net/~gadams/8203026/webrev.01/
It would be good to have a second reviewer from the serviceability
team.
Longer term it would be good to replace the rmi dependency, which
does not
appear to be central to the functionality provided here.
On 3/22/19, 9:56 AM, Roger Riggs wrote:
Hi Gary,
Holding a static reference to the implementation solves the
problem.
But I noticed that the object that is bound in the registry is
the RemoteHostImpl
and it should be the RemoteHost stub.
Line 145: should be:
bind(name.toString(), stub);
That is likely to solve the problem as well, because the
distributed garbage
collector will correctly cause a hard reference to be retained
to the implementation.
Roger
On 03/22/2019 04:05 AM, gary.ad...@oracle.com wrote:
Here's a proposed fix for the
intermittent jstatd test failure.
By moving the exported object from a local method variable to
a
static class variable a strong reference is held so the object
will not be garbage collected prematurely.
Webrev:
http://cr.openjdk.java.net/~gadams/8203026/webrev.00/
Issue: https://bugs.openjdk.java.net/browse/JDK-8203026
The test is currently filed as a core-libs/java.rmi, but it
probably
should be core-svc/tools with label=jstatd. It is the callers
responsibility to ensure the strong reference to prevent
the premature garbage collection, so this is a test failure
rather than a need to change the underlying rmi behavior.
|
