Thread: [tcljava-dev] Testcase for bug 2072924
Brought to you by:
mdejong
From: Tom P. <tpo...@ny...> - 2009-01-16 22:55:12
|
I've been looking into the bug I filed a while back, and developed a stand alone test case that illustrates the behavior. Itcl class Foo resembles my actual environment. It wraps up a Java object, and the destructor does an explicit 'rename' to get rid of the object command. Bar class is the same, except it does not explicitly rename the Java object upon destruction. Proc 'testVar' is similar to the Foo class, but obviously without Itcl class or namespace issues - it too explicitly renames the Java object to {} when going out of scope. The behavior in the bug I reported is evident when running: jaclsh testcase.tcl Foo The first time the 'testFoo' proc runs, all is good, but subsequent executions of 'testFoo' reports errors, the Java object command is returned, but it is no longer a valid Tcl command. Testing class Bar, without an explicit rename of the Java object: jaclsh testcase.tcl Bar This case runs to completion, but leaks the Java object command into the top level namespace. Furthermore, it appears that the ReflectObject.useCount is incremented for the ReflectObject, each time through 'testBar' (as witnessed with the Eclipse debugger). See below for output. Unfortunately the patch I submitted seems to also increment the useCount, therefore it too leaks objects in Java, not necessarily in Tcl command space. Mo, do you see anything wrong with my Foo class or its usage? testcase.tcl: ---cut here--------------------------------------------------------------- package require java package require Itcl # itcl class - destructor explicitly destroys the java object _obj_ itcl::class Foo { private variable _obj_ constructor {sbObj} { set _obj_ $sbObj } destructor { puts "destruct $this" catch {rename $_obj_ {}} } public method _getObj_ {} { return $_obj_ } public method getValue {} { return [$_obj_ toString] } } # itcl class - destructor does not destroy the java object _obj_ itcl::class Bar { private variable _obj_ constructor {sbObj} { set _obj_ $sbObj } destructor { puts "destruct $this" } public method _getObj_ {} { return $_obj_ } public method getValue {} { return [$_obj_ toString] } } # create a HashMap, put in one StringBuffer object set hm [java::new java.util.HashMap] set sb [java::new String sb] $hm put $sb [java::new java.lang.StringBuffer foobar] puts "info commands java0x*: [lsort [info commands java0x*]]" # proc to test retrieving the StringBuffer object into a simple Tcl variable # explicitly destroy the StringBuffer object command via "rename" proc testVar {} { global hm sb set o [$hm get $sb] puts "testVar: [$o toString]" rename $o {} return "" } # proc to test retrieving the StringBuffer object into an Itcl object Foo proc testFoo {} { global hm sb set o [$hm get $sb] set f [Foo #auto $o] puts "testFoo: [$f getValue]" itcl::delete object $f return "" } # proc to test retrieving the StringBuffer object into an Itcl object Bar proc testBar {} { global hm sb set o [$hm get $sb] set b [Bar #auto $o] puts "testBar: [$b getValue]" itcl::delete object $b return "" } if {[lindex $argv 0] eq "Foo"} { puts ==testVar=========================================== for {set i 1} {$i < 5} {incr i} { if {[catch {testVar} result]} { puts "testVar: iteration $i catch error: $result" } puts "testVar: iteration $i \[info commands java0x*\]: [lsort [info commands java0x*]]" } puts ==testFoo=========================================== for {set i 1} {$i < 5} {incr i} { if {[catch {testFoo} result]} { puts "testFoo: iteration $i catch error: $result" } puts "testFoo: iteration $i \[info commands java0x*\]: [lsort [info commands java0x*]]" } } elseif {[lindex $argv 0] eq "Bar"} { puts ==testVar=========================================== for {set i 1} {$i < 5} {incr i} { if {[catch {testVar} result]} { puts "testVar: iteration $i catch error: $result" } puts "testVar: iteration $i \[info commands java0x*\]: [lsort [info commands java0x*]]" } puts ==testBar=========================================== for {set i 1} {$i < 5} {incr i} { if {[catch {testBar} result]} { puts "testVar: iteration $i catch error: $result" } puts "testbar: iteration $i \[info commands java0x*\]: [lsort [info commands java0x*]]" } } else { error "arg should be \"Foo\" or \"Bar\"" } ---cut here--------------------------------------------------------------- Test results: $ jaclsh testcase.tcl Foo info commands java0x*: java0x1 java0x2 ==testVar=========================================== testVar: foobar testVar: iteration 1 [info commands java0x*]: java0x1 java0x2 testVar: foobar testVar: iteration 2 [info commands java0x*]: java0x1 java0x2 testVar: foobar testVar: iteration 3 [info commands java0x*]: java0x1 java0x2 testVar: foobar testVar: iteration 4 [info commands java0x*]: java0x1 java0x2 ==testFoo=========================================== testFoo: foobar destruct ::foo0 testFoo: iteration 1 [info commands java0x*]: java0x1 java0x2 testFoo: iteration 2 catch error: invalid command name "java0x8" testFoo: iteration 2 [info commands java0x*]: java0x1 java0x2 testFoo: iteration 3 catch error: invalid command name "java0x8" testFoo: iteration 3 [info commands java0x*]: java0x1 java0x2 testFoo: iteration 4 catch error: invalid command name "java0x8" testFoo: iteration 4 [info commands java0x*]: java0x1 java0x2 $ jaclsh testcase.tcl Bar info commands java0x*: java0x1 java0x2 ==testVar=========================================== testVar: foobar testVar: iteration 1 [info commands java0x*]: java0x1 java0x2 testVar: foobar testVar: iteration 2 [info commands java0x*]: java0x1 java0x2 testVar: foobar testVar: iteration 3 [info commands java0x*]: java0x1 java0x2 testVar: foobar testVar: iteration 4 [info commands java0x*]: java0x1 java0x2 ==testBar=========================================== testBar: foobar destruct ::bar0 testbar: iteration 1 [info commands java0x*]: java0x1 java0x2 java0x8 testBar: foobar destruct ::bar1 testbar: iteration 2 [info commands java0x*]: java0x1 java0x2 java0x8 testBar: foobar destruct ::bar2 testbar: iteration 3 [info commands java0x*]: java0x1 java0x2 java0x8 testBar: foobar destruct ::bar3 testbar: iteration 4 [info commands java0x*]: java0x1 java0x2 java0x8 -- Tom Poindexter tpo...@ny... |
From: Mo D. <mo...@mo...> - 2009-02-03 07:19:55
|
Tom Poindexter wrote: > I've been looking into the bug I filed a while back, and developed > a stand alone test case that illustrates the behavior. > > Itcl class Foo resembles my actual environment. It wraps up > a Java object, and the destructor does an explicit 'rename' to > get rid of the object command. > > Tom The short answer is that you have in fact found a bug. The longer answer is I need to look at it some more to really be sure the fix is right. Here is a trimmed down test case that reproduces the problem without Itcl or anything else involved. package require java # Create table, add StringBuffer to the table set table [java::new java.util.HashMap] set key_obj [java::new String fookey] set value_obj [java::new java.lang.StringBuffer fooval] $table put $key_obj $value_obj # Lookup the Java object in the table, this will return # a new ref since the return type of the method is # Object but the original reflected type was StringBuffer. set lookup_ref [$table get $key_obj] set cmd "rename $lookup_ref {}" # Evaluate this command to delete the Tcl command for # the Java object currently reflected in lookup_ref. eval $cmd # Now query the Java object in the table, this must # not return the now stale ref currently contained # in value_obj. set new_ref [$table get $key_obj] # This command will fail if an invalid ref was returned $new_ref toString BROKEN OUTPUT: $ jaclsh % package require java % set table [java::new java.util.HashMap] java0x1 % set key_obj [java::new String fookey] java0x2 % set value_obj [java::new java.lang.StringBuffer fooval] java0x3 % $table put $key_obj $value_obj java0x0 % set lookup_ref [$table get $key_obj] java0x4 % set cmd "rename $lookup_ref {}" rename java0x4 {} % eval $cmd % set new_ref [$table get $key_obj] java0x4 % $new_ref toString invalid command name "java0x4" NOT BROKEN: % set new_ref [$table get $key_obj] java0x5 % $new_ref toString fooval Now, you can get these fixed results by making the change described in this patch, but this is just a work in progress. It is for hacking purposes only! diff -u -r orig/jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java --- orig/jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java Thu Apr 13 00:36:50 2006 +++ jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java Mon Feb 2 22:38:48 2009 @@ -512,7 +512,7 @@ ReflectObject roRep = findInReflectTable(interp, cl, obj); - if (roRep != null) { + if (roRep != null && roRep.isValid) { // If it is already in the table just increment the use count and return it roRep.useCount++; I am not 100% sure about this patch, because it seems that the duplicated entries and not being cleared out of the reflect table like they should be. More on that to come. cheers Mo DeJong |
From: Tom P. <tpo...@ny...> - 2009-02-03 14:39:36
|
On Mon, Feb 02, 2009 at 11:19:52PM -0800, Mo DeJong wrote: > The short answer is that you have in fact found a bug. The longer answer > is I need to look at it some more to really be sure the fix is right. Thanks for looking into this bug! [...] > diff -u -r orig/jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java > jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java > --- orig/jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java Thu Apr [...] > I am not 100% sure about this patch, because it seems that the > duplicated entries and not being cleared out of the reflect table like > they should be. More on that to come. I agree, the patch I submitted isn't quite right and leaks Java objects. -- Tom Poindexter tpo...@ny... |
From: Tom P. <tpo...@ny...> - 2009-03-19 03:06:07
|
On Mon, Feb 02, 2009 at 11:19:52PM -0800, Mo DeJong wrote: > The short answer is that you have in fact found a bug. The longer answer > is I need to look at it some more to really be sure the fix is right. > Here is a trimmed down test case that reproduces the problem without > Itcl or anything else involved. [...] > Now, you can get these fixed results by making the change described in > this patch, but this is just a work in progress. It is for hacking > purposes only! > > diff -u -r orig/jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java > jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java > --- orig/jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java Thu Apr > 13 00:36:50 2006 > +++ jacl1.4.1/src/tcljava/tcl/lang/ReflectObject.java Mon Feb 2 > 22:38:48 2009 > @@ -512,7 +512,7 @@ > > ReflectObject roRep = findInReflectTable(interp, cl, obj); > > - if (roRep != null) { > + if (roRep != null && roRep.isValid) { > // If it is already in the table just increment the use count > and return > it > > roRep.useCount++; > > > I am not 100% sure about this patch, because it seems that the > duplicated entries and not being cleared out of the reflect table like > they should be. More on that to come. I have another patch that might work. For my use cases, it seems to avoid leaking objects. The patch is your code above, plus invoking 'dispose()' at the end of 'disposeCmd()' method: diff -r -u tcljava/src/tcljava/tcl/lang/ReflectObject.java tcljava.ae/src/tcljava/tcl/lang/ReflectObject.java --- tcljava/src/tcljava/tcl/lang/ReflectObject.java 2008-12-23 19:36:01.000000000 -0700 +++ tcljava.ae/src/tcljava/tcl/lang/ReflectObject.java 2009-03-17 21:12:57.000000000 -0600 @@ -512,7 +512,7 @@ ReflectObject roRep = findInReflectTable(interp, cl, obj); - if (roRep != null) { + if (roRep != null && roRep.isValid) { // If it is already in the table just increment the use count and return it roRep.useCount++; @@ -949,6 +949,7 @@ } isValid = false; + dispose(); } ^L -- Tom Poindexter tpo...@ny... |