> On Sat, 21 Oct 2000, Dan Wickstrom wrote:
>
> > >
> > > What does your src/tclblend/tcl/lang/CObject.java finalize()
> > > method look like? I thought the last round of patches left
> > > that ptr dangling on purpose. I have not has time to
> >
> > Yes the finalize method just calls the super class finalize method.
>
> Well, that would be your memory leak :)
I finished an initial implementation of some code
that will put an object that needs to be decr'ed
into the event queue.
I have been getting some good results, the crashing
I was seeing with JDK 1.3 from IBM has now gone
away. I also ran it with some tests under JDK 1.1
and it is working (with one small exception).
Could some folks to try this out and see if it
makes the crash in the GC thread problem go away?
There should be no more memory leaks.
The patch is for src/tclblend/tcl/lang/CObject.java
off of the contract branch.
thanks
Mo DeJong
Red Hat Inc
Index: CObject.java
===================================================================
RCS file: /cvsroot/tcljava/tcljava/src/tclblend/tcl/lang/CObject.java,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 CObject.java
--- CObject.java 1998/10/14 21:09:10 1.1.1.1
+++ CObject.java 2000/10/22 02:25:36
@@ -24,11 +24,9 @@
class CObject extends InternalRep {
-/*
- * This long really contains a Tcl_Obj*. It is declared with package
- * visibility so that subclasses that define type specific functionality
- * can get to the Tcl_Obj.
- */
+// This long really contains a Tcl_Obj*. It is declared with package
+// visibility so that subclasses that define type specific functionality
+// can get to the Tcl_Obj. This field can be read from C code.
long objPtr;
@@ -51,8 +49,7 @@
CObject()
{
- this.objPtr = newCObject(null);
- incrRefCount(objPtr);
+ this(newCObject(null));
}
/*
@@ -76,6 +73,9 @@
{
this.objPtr = objPtr;
incrRefCount(objPtr);
+
+ Notifier notifier = Notifier.getNotifierForThread(Thread.currentThread());
+ addTableRef(notifier, objPtr);
}
/*
@@ -97,6 +97,7 @@
public void
dispose()
{
+ removeTableRef(objPtr);
decrRefCount(objPtr);
objPtr = 0;
}
@@ -180,6 +181,7 @@
* newInstance --
*
* Construct a new TclObject from a Tcl_Obj*. This routine is only
+ * called from C. It is also the only CObject method that can be
* called from C.
*
* Results:
@@ -218,11 +220,13 @@
{
if (objPtr != 0) {
/*
- * If the object is finalized while the reference is still valid,
- * we need to sever the connection to the underlying Tcl_Obj*.
+ * If a CObject is finalized while the reference is still valid,
+ * we need to send the reference back to the thread that created it.
+ * We can then sever the connection to the underlying Tcl_Obj* by
+ * calling decrRefCount() in that thread.
*/
- decrRefCount(objPtr);
+ finalizeTableRef(objPtr);
}
super.finalize();
}
@@ -329,6 +333,202 @@
makeRef(
long objPtr, // Pointer to Tcl_Obj.
TclObject object); // Object that Tcl_Obj should refer to.
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * CleanupTableEntry --
+ *
+ * This is a little helper class used to store the objPtr
+ * and to take on the role of the TclEvent that will be
+ * called after the event is queued.
+ *
+ * Results:
+ * None.
+ *
+ * Side effects:
+ * None.
+ *
+ *----------------------------------------------------------------------
+ */
+
+static class CleanupTableEntry extends TclEvent {
+ Notifier notifier;
+ long objPtr;
+
+ CleanupTableEntry(Notifier notifier, long objPtr) {
+ this.notifier = notifier;
+ this.objPtr = objPtr;
+ }
+
+ public int processEvent(int flags) {
+ System.err.println("calling decrRefCount from processEvent for " + objPtr);
+ CObject.decrRefCount(objPtr);
+ return 1;
+ }
+
+ // This method will add this TclEvent to the
+ // notifier for the Interp that created the
+ // original Tcl_Obj ptr. This must be done
+ // asyncronously, we can't block the GC
+ // thread for any reason!
+
+ void appendToNotifierQueue() {
+ System.err.println("queueing cleanup for " + objPtr);
+ notifier.queueEvent(this, TCL.QUEUE_TAIL);
+ }
+}
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * addTableRef --
+ *
+ * Create a ref in the global Tcl_Obj table. It is used to
+ * keep track of the interp where a given object was created
+ * so that we can queue up an event in that interp.
+ *
+ * Results:
+ * None.
+ *
+ * Side effects:
+ * None.
+ *
+ *----------------------------------------------------------------------
+ */
+
+private static void
+addTableRef(
+ Notifier notifier, // Notifier for current thread
+ long objPtr) // The Tcl_Obj to cleanup
+{
+ // Add to table in thread safe way.
+
+ synchronized (cleanupTable) {
+ CleanupTableEntry te = new CleanupTableEntry(notifier, objPtr);
+
+ // Look for a free slot in the table.
+
+ for (int i = 0; i < cleanupTable.length; i++) {
+ if (cleanupTable[i] == null) {
+ cleanupTable[i] = te;
+ return;
+ }
+ }
+
+ // If the table is too small, we need to resize it.
+ // We can't synchronize access to the table inside
+ // finalizeTableRef so we need to be extra careful.
+ // Replace the cleanupTable pointer after the
+ // new one is ready, but be sure not to change the
+ // contents of the old table in case it is getting
+ // used by the GC thread in finalizeTableRef.
+
+ CleanupTableEntry[] newArr =
+ new CleanupTableEntry[(int) (cleanupTable.length * 1.5)];
+
+ System.arraycopy(cleanupTable, 0, newArr, 0, cleanupTable.length);
+ newArr[cleanupTable.length] = te;
+ cleanupTable = newArr;
+ }
+}
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * removeTableRef --
+ *
+ * Remove the given entry from the table. This method
+ * is invoked in the case where the object was
+ * explicitly freed by a call to release().
+ *
+ * Results:
+ * None.
+ *
+ * Side effects:
+ * None.
+ *
+ *----------------------------------------------------------------------
+ */
+
+private static void
+removeTableRef(
+ long objPtr) // The Tcl_Obj to cleanup
+{
+ // Remove from the ref table in a thread safe way.
+
+ synchronized (cleanupTable) {
+ for (int i = 0; i < cleanupTable.length; i++) {
+ CleanupTableEntry te = cleanupTable[i];
+ if ((te != null) && (objPtr == te.objPtr)) {
+ cleanupTable[i] = null;
+ return;
+ }
+ }
+ }
+}
+
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * finalizeTableRef --
+ *
+ * This method is called by the GC thread when
+ * it finds a dangling objPtr inside a CObject.
+ * This method will add queue up an event that
+ * will release this dangling objPtr.
+ *
+ * Results:
+ * None.
+ *
+ * Side effects:
+ * None.
+ *
+ *----------------------------------------------------------------------
+ */
+
+private static void
+finalizeTableRef(
+ long objPtr) // The Tcl_Obj to cleanup
+{
+ // We can't synchronize access to this table from
+ // inside the GC thread, so we need to be extra
+ // careful. Grab a ref to the cleanup table and
+ // then use that ref instead of the original
+ // table ref. It is ok if the cleanupTable
+ // gets replaced with a new array after this
+ // because localTable will have a ref to
+ // the original array.
+
+ CleanupTableEntry[] localTable = cleanupTable;
+
+ for (int i = 0; i < localTable.length; i++) {
+ if (objPtr == localTable[i].objPtr) {
+ localTable[i].appendToNotifierQueue();
+
+ // Now free the slot in the original
+ // cleanupTable, if the table was changed
+ // after we grabbed it, this will free
+ // the slot in the new table. Otherwise
+ // localTable and cleanupTable are the same.
+
+ cleanupTable[i] = null;
+ return;
+ }
+ }
+
+ System.err.println("Entry in cleanup table not found for " + objPtr);
+ throw new RuntimeException("Entry in cleanup table not found for " + objPtr);
+}
+
+// This table holds one entry for the objPtr of each CObject in the system.
+// It is used to map from a objPtr to the notifier for the thread where
+// the object was created. This data is used by the GC thread to finalize.
+
+// FIXME : how big should this be ?
+
+private static CleanupTableEntry[] cleanupTable = new CleanupTableEntry[100];
} // end CObject
