[Tcl Java] Re: possible memory.
On Thu, 26 Oct 2000, Daniel Wickstrom wrote: "Mo" == Mo DeJong [EMAIL PROTECTED] writes: Mo Are you up to it? I am going to be working on this object ref Mo queue thing for some time, so if you could take a whack at the Mo Notifier it would really help. There is not going to be much Mo overlap in these changes, so they can be done in parallel. Ok here is a patch to make the notifier thread safe. I've run the test cases on a solaris box, and it still fails with the same test cases that I posted earlier before I made the modifications. Let me know what you think. This patch looks great. I touched up a couple of little things and checked it in (see the ChangeLog for the gory details). I ran the tests again, and it looks like everything is working just great. I can also run some multi-threaded tests that did not work in the past. Are there any remaining issues we need to look at before merging this branch back into the HEAD? We can't fix the ref count problem right now, we need to get this all merged back into the HEAD and then we can look at overhauling ref counting, perhaps on another branch. Mo DeJong Red Hat Inc The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com
[Tcl Java] Re: possible memory.
"Mo" == Mo DeJong [EMAIL PROTECTED] writes: Mo I ran the tests again, and it looks like everything is working Mo just great. I can also run some multi-threaded tests that did Mo not work in the past. Mo Are there any remaining issues we need to look at before Mo merging this branch back into the HEAD? We can't fix the ref Mo count problem right now, we need to get this all merged back Mo into the HEAD and then we can look at overhauling ref Mo counting, perhaps on another branch. This sounds promising. As far as threading changes, there was the issue of thread cleanup routine not being called. Other than that and the ref counting changes that you mentioned, I thinks that's it. If I get a chance this weekend, I'll spend some time looking at the thread cleanup issue. -Dan The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com
[Tcl Java] Re: possible memory.
"Mo" == Mo DeJong [EMAIL PROTECTED] writes: Mo Are you up to it? I am going to be working on this object ref Mo queue thing for some time, so if you could take a whack at the Mo Notifier it would really help. There is not going to be much Mo overlap in these changes, so they can be done in parallel. Ok here is a patch to make the notifier thread safe. I've run the test cases on a solaris box, and it still fails with the same test cases that I posted earlier before I made the modifications. Let me know what you think. How is it going with the object ref queue thing? -Dan notifier-patch.txt
[Tcl Java] Re: possible memory.
That may be true for your specific case. Though, it is not true in general. One could also potentially delete the interpreter without removing the thread. For example, one may be using a pool of Java threads to do some work. One thread is retrieved, creates a Tcl interp, do some work, delete the interp, goes back into the thread pool. The thread itself is never deleted, only the resources used by the thread. TclBlend allows a Tcl program to use Java. It also needs to work the other direction, which is allowing a Java program to use Tcl. -- Jiang Wu [EMAIL PROTECTED] -Original Message- From: Dan Wickstrom [mailto:[EMAIL PROTECTED]] Sent: Sunday, October 22, 2000 11:53 AM To: [EMAIL PROTECTED] Cc: [EMAIL PROTECTED] Subject: [Tcl Java] Re: possible memory. Jiang Wu wrote: Let's not forget there is a problem with the putting object on a queue for deletion by the interpreter. As the GC may run at a random time, it is possible when a Cobject is being finalized, the associated interpreter is already deleted. 1. Create a thread, 2. Add a Tcl interpreter 3. Do some Tcl work 4. Delete the Tcl interpreter 5. Delete the thread 6. GC runs to finalize objects created in the above process It seems that we're preventing this by having the thread cleanup routine detach the jvm thread. All of the java objects would be finalized when the thread was detached. Of course, maybe I don't understand what you mean. In aolserver, the case that you're talking about would never happen. -Dan The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com
[Tcl Java] Re: possible memory.
On Tue, 24 Oct 2000, Jiang Wu wrote: That may be true for your specific case. Though, it is not true in general. One could also potentially delete the interpreter without removing the thread. For example, one may be using a pool of Java threads to do some work. One thread is retrieved, creates a Tcl interp, do some work, delete the interp, goes back into the thread pool. The thread itself is never deleted, only the resources used by the thread. TclBlend allows a Tcl program to use Java. It also needs to work the other direction, which is allowing a Java program to use Tcl. -- Jiang Wu [EMAIL PROTECTED] Yeah, that is a nasty one. We really need to create a good document that details all of these create/destroy cases. In this case, the thread would hold the same notifier but the interp it was create in would be different. I am not sure if this would blow things up. It might be enough to put a call to System.gc() in the Interp destroy callback. Not only do we need to doc this, but we need regression tests for these edge cases too. I have been thinking about how to include thread tests in the existing regression test suite, I think a --with-thread option that points to where the Thread extension is build is the only way to go. Mo DeJong Red Hat Inc The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com
[Tcl Java] Re: possible memory.
Mo DeJong wrote:
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 realize that now, but I think the idea behind Jiang's reference counting changes
was to explicitly increment/decrement the reference count as needed similar to what
is done in the native tcl code. I think there are still quite a few things that
need to be fixed to make this work, because the current code makes alot of
assumptions based on the fact that the CObject finalizer takes care of the Tcl_Obj
cleanup. I think that it's still doable, but probably alot more work would be
needed to make the reference counting work right.
Yes. The tricky part is going to be changing the C code to support
this. We are going to need to have the ability to enque a TclEvent
in the GC thread, but we need an interp handle for that.
Here is that I am currently looking into:
A CObject() is created by calling
newInstance() from JNI code.
But, we don't have a handle to
an interp in the calling code.
We need to wrap this into an
object along with the interp
handle and put it into a table
somewhere so that the wrapped
object can not get GCed.
class CObjectCleanupTable {
static void addObjRef(Interp i, long ptr)
static void deleteObjRef(long ptr)
static void finalizeObjRef(long ptr)
}
It seems like we could change the
CObject.newInstance() signature, so
that the Interp object would need
to get passed along too.
(Old)
private static TclObject
newInstance(
long objPtr)// Tcl_Obj to wrap.
{
return new TclObject(new CObject(objPtr));
}
(New)
private static TclObject
newInstance(
Interp interp, // Current Interp
long objPtr)// Tcl_Obj to wrap.
{
return new TclObject(new CObject(interp, objPtr));
}
This means that any place where CObject.newInstance()
(only JavaGetTclObject at this point) is called would
also need to include the Interp parameter.
Now the CObject ctor could register the given objPtr
like so:
CObject(
Interp interp, // The current Interp
long objPtr)// Pointer to Tcl_Obj from C.
{
this.objPtr = objPtr;
incrRefCount(objPtr);
CObjectCleanupTable.addObjRef(interp, objPtr);
}
Then in the finalize method we would do this:
protected void finalize() throws Throwable
{
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*.
*/
CObjectCleanupTable.finalizeObjRef(objPtr);
}
super.finalize();
}
That would enqueue a TclEvent that would end up calling
CObject.decrRefCount(objPtr) inside the thread where
the Tcl_Obj was created.
I think it is doable, but some hacking is going to
be required.
It seems like it would work.
We also need to make the Tcl Blend Notifier thread
safe. I think time would be better spent focusing
on that right now instead of fighting ref counting.
Is this just a matter of modifying the Notifier class so that there
is an event queue per thread instead of a global queue as there
is now. Are there any modifications necessary in the 'c' code?
Making the Notifier "thread safe" is actually the easy part.
All you would need to do is add another field to the Java
interp structure that would store the "Tcl Thread ID" value.
You would get the thread id in the pkg init code, and then save
it into the Interp structure.
ThreadID curThread;
GetCurrentThread(curThread);
/* put into interp field */
This TclThreadID is needed so that you can call the
Tcl_ThreadAlert(ThreadID) from inside the Notifier
code.
Once that bit is in place, you would then need to
rip out the current notifier implementation.
Start with the Jacl impl of Notifier and
copy it into the Tcl Blend dir. Now merge any
diffs over from the Tcl Blend impl but leave
out all of the "global notifier" crap. The
only thing to watch out for is to keep
Jiang's fixed for deadlocking in the
Notifier queue. I don't think that
patch ever made it into Jacl so
don't copy that bit (see ChangeLog
entry on 2000-08-23).
Now it is just a matter of going in a deleting
all the unused C code that was used to implement
the old notifier. That part should be fun cause
there is just a gob of nasty code in there and
one quick press of the delete button will finish
it off.
Are you up to it? I am going to be working on
this object ref queue thing for some time,
so if you could take a whack at the Notifier
it would really help. There is not going to
be much overlap in these changes, so they
can be done in parallel.
I'll see if
[Tcl Java] Re: possible memory.
On Sun, 22 Oct 2000, Jiang Wu wrote: Let's not forget there is a problem with the putting object on a queue for deletion by the interpreter. As the GC may run at a random time, it is possible when a Cobject is being finalized, the associated interpreter is already deleted. 1. Create a thread, 2. Add a Tcl interpreter 3. Do some Tcl work 4. Delete the Tcl interpreter 5. Delete the thread 6. GC runs to finalize objects created in the above process I agree that the case you describe is a concern, but I don't see that we have a choice at this point. I would like to get this free with the queue thing and fixing up of the Notifier done, and then worry about overhauling the ref counting approach. We need to get these changes finished off and merged back into the HEAD very soon. Currently, we have no thread safe code to point people to. Something that works for 99.999% of the cases is better than something that works for 0% of them. Mo DeJong Red Hat Inc The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com
[Tcl Java] Re: possible memory.
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.java1998/10/14 21:09:10 1.1.1.1
+++ CObject.java2000/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
[Tcl Java] Re: possible memory.
On Fri, 20 Oct 2000, Daniel Wickstrom wrote: The TclList.getLength method converts indexListObj to a TclList object which uses an underlying Tcl_Obj to hold the internal rep. At the end of this case statement, it returns leaving indexListObj with no references. The jvm garbage collects indexListObj, but the underlying Tcl_Obj is left stranded and results in a memory leak. -Dan 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 look at the patches your guys are using so I am not sure what is going on. I think what we need to do in the short term is to implement a patch that will send a Tcl_Obj that gets freed back to the Tcl interp using the thread safe event queue. That way we can free the object up back in the Tcl thread and it will not core dump like it would if we freed it up in the JVM GC thread. After we have something (anything) working we can hack around with the various ref count solutions to see if we can find one that passes all the tests and does not leak memory. We also need to make the Tcl Blend Notifier thread safe. I think time would be better spent focusing on that right now instead of fighting ref counting. Mo DeJong Red Hat Inc The TclJava mailing list is sponsored by Scriptics Corporation. To subscribe:send mail to [EMAIL PROTECTED] with the word SUBSCRIBE as the subject. To unsubscribe: send mail to [EMAIL PROTECTED] with the word UNSUBSCRIBE as the subject. To send to the list, send email to '[EMAIL PROTECTED]'. An archive is available at http://www.mail-archive.com/tcljava@scriptics.com
[Tcl Java] Re: possible memory.
The last posting got mangled a little, so I'm attaching the trace file. It's probably a lot clearer than looking at my last posting. -Dan 0 ckalloc 8c5d99024./../generic/tclLiteral.c 261 1 ckalloc 8c5d9d817./../generic/tclLiteral.c 267 2 ckalloc 88f783812./../generic/tclLiteral.c 290 3 ckalloc 8c5da2029./../generic/tclCompile.c 1737 4 ckalloc 8c5da7024./../generic/tclLiteral.c 261 5 ckalloc 8c5dab810./../generic/tclLiteral.c 267 6 ckalloc 88f23e012./../generic/tclLiteral.c 290 7 ckalloc 88f4ec024./../generic/tclExecute.c 4863: freed at 117 8 ckalloc 88f4f0824./../generic/tclLiteral.c 261 9 ckalloc 88f4f5021./../generic/tclLiteral.c 267 10 ckalloc 88f4f9812./../generic/tclLiteral.c 290 11 ckalloc 88f4fd831./../generic/tclCompile.c 1737 12 ckalloc 86c830824./../generic/tclLiteral.c 261 13 ckalloc 86c8350 9./../generic/tclLiteral.c 267 14 ckalloc 86c839012./../generic/tclLiteral.c 290 15 ckalloc 86c83d024./../generic/tclLiteral.c 261 16 ckalloc 86c8418 4./../generic/tclLiteral.c 267 17 ckalloc 86c845012./../generic/tclLiteral.c 290 18 ckalloc 86a780024./../generic/tclLiteral.c 261 19 ckalloc 86a784810./../generic/tclLiteral.c 267 20 ckalloc 86a788812./../generic/tclLiteral.c 290 21 ckalloc 86a78c824./../generic/tclLiteral.c 261 22 ckalloc 86a7910 6./../generic/tclLiteral.c 267 23 ckalloc 86a794812./../generic/tclLiteral.c 290 24 ckalloc 8c5c51864./../generic/tclLiteral.c 845: freed at 75 25 ckalloc 8c5c58830./../generic/tclCompile.c 1737 26 ckalloc 8c5c5d824./../generic/tclLiteral.c 261 27 ckalloc 8c5c62011./../generic/tclLiteral.c 267 28 ckalloc 8c5c66012./../generic/tclLiteral.c 290 29 ckalloc 8c55ad828./../generic/tclCompile.c 1737 30 ckalloc 8c55b2824./../generic/tclLiteral.c 261 31 ckalloc 8c55b70 9./../generic/tclLiteral.c 267 32 ckalloc 8c55bb012./../generic/tclLiteral.c 290 33 ckalloc 8c55bf024./../generic/tclLiteral.c 261 34 ckalloc 8c55c3816./../generic/tclLiteral.c 267 35 ckalloc 86abba012./../generic/tclLiteral.c 290 36 ckalloc 86abbe026./../generic/tclCompile.c 1737 37 ckalloc 86abc3024./../generic/tclLiteral.c 261 38 ckalloc 86abc7812./../generic/tclLiteral.c 267 39 ckalloc 86abcb812./../generic/tclLiteral.c 290 40 ckalloc 86abcf824./../generic/tclLiteral.c 261 41 ckalloc 868bca011./../generic/tclLiteral.c 267 42 ckalloc 868bce012./../generic/tclLiteral.c 290 43 ckalloc 868bd2024./../generic/tclExecute.c 4863: freed at 586 44 ckalloc 868bd6824./../generic/tclLiteral.c 261 45 ckalloc 868bdb020./../generic/tclLiteral.c 267 46 ckalloc 868bdf812./../generic/tclLiteral.c 290 47 ckalloc 868be3828./../generic/tclCompile.c 1737 48 ckalloc 86a28d824./../generic/tclLiteral.c 261 49 ckalloc 8c5c6a0 5./../generic/tclLiteral.c 267 50 ckalloc 86a292012./../generic/tclLiteral.c 290 51 ckalloc 86a296027./../generic/tclCompile.c 1737 52 ckalloc 86a29b030./../generic/tclCompile.c 1737 53 ckalloc 86a2a0024./../generic/tclLiteral.c 261 54 ckalloc 86a2a48 4./../generic/tclLiteral.c 267 55 ckalloc 86a2a8012./../generic/tclLiteral.c 290 56 ckalloc 86a620824./../generic/tclLiteral.c 261 57 ckalloc 86a6250 5./../generic/tclLiteral.c 267 58 ckalloc 86a628812./../generic/tclLiteral.c 290 59 ckalloc 86a62c824./../generic/tclLiteral.c 261 60 ckalloc 86a6310 9./../generic/tclLiteral.c 267 61 ckalloc 86a635012./../generic/tclLiteral.c 290 62 ckalloc 86a639024./../generic/tclLiteral.c 261 63
