From d1e9e61819c4e3bcb031a31d2c20fbf650c62ee0 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Wed, 03 Jul 2013 15:22:12 +0000
Subject: [PATCH] Fix OPENDJ-1049: ReferenceCountedObject can be released twice in some circumstances
---
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/util/ReferenceCountedObject.java | 11 +++--
opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/util/ReferenceCountedObjectTestCase.java | 58 +++++++++++++++++------------
2 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/util/ReferenceCountedObject.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/util/ReferenceCountedObject.java
index 95457c5..0ddc8d5 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/util/ReferenceCountedObject.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/util/ReferenceCountedObject.java
@@ -25,6 +25,7 @@
package com.forgerock.opendj.util;
+
/**
* An object which is lazily created when first referenced, and destroyed when
* the last reference is released.
@@ -72,9 +73,12 @@
public void release() {
T instanceToRelease = null;
synchronized (lock) {
- if (value != null && instance == value && --refCount == 0) {
- instanceToRelease = value;
- instance = null;
+ if (value != null) {
+ if (instance == value && --refCount == 0) {
+ // This was the last reference.
+ instanceToRelease = value;
+ instance = null;
+ }
/*
* Force NPE for subsequent get() attempts and prevent
@@ -83,7 +87,6 @@
value = null;
}
}
-
if (instanceToRelease != null) {
destroyInstance(instanceToRelease);
}
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/util/ReferenceCountedObjectTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/util/ReferenceCountedObjectTestCase.java
index ec51fd9..7fde80c 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/util/ReferenceCountedObjectTestCase.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/util/ReferenceCountedObjectTestCase.java
@@ -91,36 +91,46 @@
}
/**
- * This test attempts to test that finalization works. It loops at most 100
- * times performing GCs and checking to see if the finalizer was called.
- * Usually objects are finalized after 2 GCs, so the loop should complete
- * quite quickly.
- *
- * @throws Exception
- * If an unexpected error occurred.
+ * This test attempts to test that finalization works.
*/
@Test
- public void testFinalization() throws Exception {
+ public void testFinalization() {
final Impl impl = mock(Impl.class);
when(impl.newInstance()).thenReturn(object);
final ReferenceCountedObject<Object> rco = rco(impl);
- ReferenceCountedObject<Object>.Reference ref = rco.acquire();
- System.gc();
- System.gc();
+ final ReferenceCountedObject<Object>.Reference ref = rco.acquire();
verify(impl, never()).destroyInstance(object);
- // Read in order to prevent optimization.
- if (ref != null) {
- ref = null;
- }
- for (int i = 0; i < 100; i++) {
- System.gc();
- try {
- verify(impl).destroyInstance(object);
- break; // Finalized so stop.
- } catch (final Throwable t) {
- // Retry.
- }
- }
+ ref.finalize(); // Simulate GC.
+ verify(impl).destroyInstance(object);
+ }
+
+ /**
+ * Test for issue OPENDJ-1049.
+ */
+ @Test
+ public void testReleaseOnceOnly() {
+ final Impl impl = mock(Impl.class);
+ when(impl.newInstance()).thenReturn(object);
+ final ReferenceCountedObject<Object> rco = rco(impl);
+
+ // Create two references.
+ final ReferenceCountedObject<Object>.Reference ref1 = rco.acquire();
+ final ReferenceCountedObject<Object>.Reference ref2 = rco.acquire();
+
+ /*
+ * Now release first reference multiple times and make sure that the
+ * resource is not destroyed.
+ */
+ ref1.release();
+ ref1.release(); // Redundant release.
+ ref1.finalize(); // Simulate GC.
+ verify(impl, never()).destroyInstance(object);
+
+ /*
+ * Now release second reference which should cause the resource to be
+ * destroyed.
+ */
+ ref2.release();
verify(impl).destroyInstance(object);
}
--
Gitblit v1.10.0