Add DualState.SingleDeleteGlobalRefP

The caching of a Lookup object at a PostgreSQL call site (in
flinfo->fn_extra) will call for a flavor of DualState that can
delete a JNI global reference when the call site's memory context
is reset or deleted.

This seems to be the first DualState flavor whose nativeStateReleased
method actually does anything, which requires a little refinement of
some only-when-assertions-enabled checking that had never had to
deal with that before.

Author: jcflack

pljava-so/src/main/c/DualState.c

@@ -20,6 +20,7 @@
 #include "org_postgresql_pljava_internal_DualState_SingleSPIfreetuptable.h"
 #include "org_postgresql_pljava_internal_DualState_SingleSPIcursorClose.h"
 #include "org_postgresql_pljava_internal_DualState_BBHeapFreeTuple.h"
+#include "org_postgresql_pljava_internal_DualState_SingleDeleteGlobalRefP.h"
 #include "pljava/DualState.h"
 
 #include "pljava/Exception.h"
@@ -156,6 +157,16 @@ void pljava_DualState_initialize(void)
 		{ 0, 0, 0 }
 	};
 
+	JNINativeMethod deleteGlobalRefPMethods[] =
+	{
+		{
+		"_deleteGlobalRefP",
+		"(J)V",
+		Java_org_postgresql_pljava_internal_DualState_00024SingleDeleteGlobalRefP__1deleteGlobalRefP
+		},
+		{ 0, 0, 0 }
+	};
+
 	s_DualState_class = (jclass)JNI_newGlobalRef(PgObject_getJavaClass(
 		"org/postgresql/pljava/internal/DualState"));
 	s_DualState_cleanEnqueuedInstances = PgObject_getStaticJavaMethod(
@@ -206,6 +217,11 @@ void pljava_DualState_initialize(void)
 	PgObject_registerNatives2(clazz, bbHeapFreeTupleMethods);
 	JNI_deleteLocalRef(clazz);
 
+	clazz = (jclass)PgObject_getJavaClass(
+		"org/postgresql/pljava/internal/DualState$SingleDeleteGlobalRefP");
+	PgObject_registerNatives2(clazz, deleteGlobalRefPMethods);
+	JNI_deleteLocalRef(clazz);
+
 	/*
 	 * Call initialize() methods of known classes built upon DualState.
 	 */
@@ -418,3 +434,23 @@ Java_org_postgresql_pljava_internal_DualState_00024BBHeapFreeTuple__1heapFreeTup
 	heap_freetuple(tup);
 	END_NATIVE
 }
+
+
+
+/*
+ * Class:     org_postgresql_pljava_internal_DualState_SingleDeleteGlobalRefP
+ * Method:    _deleteGlobalRefP
+ * Signature: (J)V
+ */
+JNIEXPORT void JNICALL
+Java_org_postgresql_pljava_internal_DualState_00024SingleDeleteGlobalRefP__1deleteGlobalRefP(
+	JNIEnv* env, jobject _this, jlong refp)
+{
+	Ptr2Long p2l;
+	jobject ref;
+	p2l.longVal = refp;
+	ref = *(jobject *)p2l.ptrVal;
+	*(jobject *)p2l.ptrVal = NULL;
+	/* no call into PostgreSQL here, just one simple JNI operation */
+	(*env)->DeleteGlobalRef(env, ref);
+}

pljava/src/main/java/org/postgresql/pljava/internal/DualState.java

@@ -528,27 +528,42 @@ private boolean hasPin(DualState<?> s)
 	/** Thread local record of when the PG thread is invoking callbacks. */
 	private static final CleanupTracker s_inCleanup = new CleanupTracker();
 
+	/** Flag for cleanup entry/exit from the Java side */
+	private static final int CLEAN_JAVA   = 1;
+
+	/** Flag for cleanup entry/exit from the native side */
+	private static final int CLEAN_NATIVE = 2;
+
 	/** Thread local boolean with pairing enter/exit operations. */
-	static final class CleanupTracker extends ThreadLocal<Boolean>
+	static final class CleanupTracker extends ThreadLocal<Integer>
 	{
-		boolean enter()
+		boolean enter(int how)
 		{
 			assert Backend.threadMayEnterPG() : m("inCleanup.enter thread");
-			assert ! inCleanup() : m("inCleanup.enter re-entered");
-			set(Boolean.TRUE);
+			int got = get();
+			assert (got&3) != how : m("inCleanup.enter re-entered same how");
+			assert got < 4 : m("inCleanup.enter too many entries");
+			set((got << 2) | how);
 			return true;
 		}
 
-		boolean exit()
+		boolean exit(int how)
 		{
-			assert inCleanup() : m("inCleanup.exit mispaired");
-			set(Boolean.FALSE);
+			int got = get();
+			assert (got&3) == how : m("inCleanup.exit mispaired");
+			set(got >>> 2);
 			return true;
 		}
 
 		boolean inCleanup()
 		{
-			return Boolean.TRUE == get();
+			return 0 != get();
+		}
+
+		@Override
+		protected Integer initialValue()
+		{
+			return 0;
 		}
 	}
 
@@ -1757,35 +1772,44 @@ private static void lifespanRelease(Lifespan lifespan)
 
 		DualState t = head.m_next;
 		head.m_prev = head.m_next = head;
-		for ( DualState s = t ; s != head ; s = t )
+
+		assert s_inCleanup.enter(CLEAN_NATIVE); //no-op when assertions disabled
+		try
 		{
-			t = s.m_next;
-			s.m_prev = s.m_next = null;
-			++ total;
-			/*
-			 * This lock() is part of DualState's contract with clients.
-			 * They are responsible for pinning the state instance
-			 * whenever they need the wrapped native state (which is verified
-			 * to still be valid at that time) and for the duration of whatever
-			 * operation needs access to that state. Taking this lock here
-			 * ensures the native state is blocked from vanishing while it is
-			 * actively in use.
-			 */
-			int state = s.lock(false);
-			try
+			for ( DualState s = t ; s != head ; s = t )
 			{
-				if ( z(NATIVE_RELEASED & state) )
+				t = s.m_next;
+				s.m_prev = s.m_next = null;
+				++ total;
+				/*
+				 * This lock() is part of DualState's contract with clients.
+				 * They are responsible for pinning the state instance whenever
+				 * they need the wrapped native state (which is verified to
+				 * still be valid at that time) and for the duration of whatever
+				 * operation needs access to that state. Taking this lock here
+				 * ensures the native state is blocked from vanishing while it
+				 * is actively in use.
+				 */
+				int state = s.lock(false);
+				try
 				{
-					++ release;
-					s.nativeStateReleased(
-						z(JAVA_RELEASED & state)  &&  null != s.referent());
+					if ( z(NATIVE_RELEASED & state) )
+					{
+						++ release;
+						s.nativeStateReleased(
+							z(JAVA_RELEASED & state)  &&  null != s.referent());
+					}
+				}
+				finally
+				{
+					s.unlock(state, true);//true->ensure NATIVE_RELEASED is set.
 				}
-			}
-			finally
-			{
-				s.unlock(state, true); // true -> ensure NATIVE_RELEASED is set.
 			}
 		}
+		finally
+		{
+			assert s_inCleanup.exit(CLEAN_NATIVE);
+		}
 
 		s_stats.lifespanPoll(release, total);
 	}
@@ -1807,7 +1831,7 @@ private static void cleanEnqueuedInstances()
 		int nDeferred = s_deferredReleased.size();
 		boolean isDeferred;
 
-		assert s_inCleanup.enter(); // no-op when assertions disabled
+		assert s_inCleanup.enter(CLEAN_JAVA); // no-op when assertions disabled
 		try
 		{
 			for ( ;; )
@@ -1848,7 +1872,7 @@ else if ( z(NATIVE_RELEASED & state) )
 		}
 		finally
 		{
-			assert s_inCleanup.exit();
+			assert s_inCleanup.exit(CLEAN_JAVA);
 		}
 
 		s_stats.referenceQueueDrain(total - release, release, total, reDefer);
@@ -2363,6 +2387,55 @@ protected void javaStateUnreachable(boolean nativeStateLive)
 		private native void _heapFreeTuple(ByteBuffer tuple);
 	}
 
+	/**
+	 * A {@code DualState} subclass whose only native resource releasing action
+	 * needed is a JNI {@code DeleteGlobalRef} of a single pointer.
+	 */
+	public static abstract class SingleDeleteGlobalRefP<T>
+	extends SingleGuardedLong<T>
+	{
+		protected SingleDeleteGlobalRefP(
+			T referent, Lifespan span, long dgrTarget)
+		{
+			super(referent, span, dgrTarget);
+		}
+
+		@Override
+		public String formatString()
+		{
+			return "%s DeleteGlobalRef(%x)";
+		}
+
+		/**
+		 * When the Java state is released (it won't normally go unreachable,
+		 * because of the global ref), a JNI {@code DeleteGlobalRef} call
+		 * is made so the instance can be collected without having to wait
+		 * for release of its containing context.
+		 */
+		@Override
+		protected void javaStateReleased(boolean nativeStateLive)
+		{
+			assert Backend.threadMayEnterPG();
+			if ( nativeStateLive )
+				_deleteGlobalRefP(guardedLong());
+		}
+
+		/**
+		 * When the native state is released, a JNI {@code DeleteGlobalRef} call
+		 * is made so the global ref stored in that to-be-released memory isn't
+		 * leaked (left permanently live).
+		 */
+		@Override
+		protected void nativeStateReleased(boolean javaStateLive)
+		{
+			assert Backend.threadMayEnterPG();
+			if ( javaStateLive )
+				_deleteGlobalRefP(guardedLong());
+		}
+
+		private native void _deleteGlobalRefP(long pointer);
+	}
+
 	/**
 	 * Bean exposing some {@code DualState} allocation and lifecycle statistics
 	 * for viewing in a JMX management client.