Fix(Memory Leak): Remove FencedLockProxy upon FencedLock#destroy [HZ-3039]

[gbarnett-hz]

Removes the FencedLockProxy from LockService.proxies after calling legacy semantics of AbstractBlockingService#destroyRaftObject. Previously the FencedLockProxy would be resident in LockService.proxies until the CP system was reset.

Fixes #25344

Backport of: INSERT_LINK_TO_THE_ORIGINAL_PR_HERE

EE PR: INSERT_LINK_TO_THE_EE_PR_HERE

Breaking changes (list specific methods/types/messages):

• API
• client protocol format
• serialized form
• snapshot format

Checklist:

• Labels (Team:, Type:, Source:, Module:) and Milestone set
• Label Add to Release Notes or Not Release Notes content set
• Request reviewers if possible
• Send backports/forwardports if fix needs to be applied to past/future releases
• New public APIs have @Nonnull/@Nullable annotations
• New public APIs have @since tags in Javadoc

[gbarnett-hz]

@arodionov if you can take a look -- if it seems reasonable I will add some testing post-comments.

[arodionov]

Thanks for the PR, Granville.
But, I wonder if it makes sense to even try to remove destroyed proxy objects from the map.

Map with lock proxies created on the server side (if using Hazecast in an embedded mode) or on the client side.
I assume we have such maps only for Lock proxies because: the Lock and a proxy themselves are heavy objects, and the Lock best practice use case, is to try to reuse an existing Lock object.

When the same lock is used from several Hazelcast clients or embedded instances, it means that every client/instance will have a such proxy and this proxy will be added to the maps on each instance:

HazelcastInstance instance1 = Hazelcast.newHazelcastInstance(config);
HazelcastInstance instance2 = Hazelcast.newHazelcastInstance(config);
FencedLock lockInstance1 = instance.getCPSubsystem().getLock("iLock"); // iLock proxy added to proxies Map on lockInstance1
FencedLock lockInstance2 = instance.getCPSubsystem().getLock("iLock"); // iLock proxy added to proxies Map on lockInstance2

Therefore, if we decide to remove the destroyed lock from the proxy's map, it means that it can be removed only from the instance from which the "destroy" method was called.

lockInstance1.destroy(); // iLock proxy removed only from proxies Map on lockInstance1

Other clients/instances will be unaware that the lock has been destroyed.

So, I am wondering, does it make sense to provide a "partial" cleaning at all?

[gbarnett-hz]

@arodionov I agree -- even with client changes you only ever cleanup the client that called the destroy. On the server though this will clean up the mapping that is maintained across all instances, so perhaps that's the only place it makes sense?

I had a look at removing the lock proxy in the Python client -- it looks simple but I also had same thought w.r.t. only fixing the client that actually called the destroy.

[arodionov]

Yes, for the embedded mode (server-side) proxies, I think we can proceed with removing the proxy from the proxies map.
In that case, we have a DestroyRaftObjectOp that will be executed at all instances. So we can wipe the proxies map in a consistent way.

Please try to add some tests for the current PR.

[arodionov]

@gbarnett-hz The ReflectionUtils#getFieldValueReflectively may be useful for the test.

[gbarnett-hz]

Cheers @arodionov -- will take a look

[arodionov]

Looks good to me!
Thanks, Granville!

[vbekiaris]

thanks for the fix @gbarnett-hz

[vbekiaris]

nit: I appreciate the try-finally style, but what is the point when there is no statement in the try block?