Fixed crash, minor optimizations, less allocations. (#43)

jnim.nimble

@@ -1,6 +1,6 @@
 # Package
 
-version       = "0.3.2"
+version       = "0.3.3"
 author        = "Anatoly Galiulin"
 description   = "Java bridge for Nim"
 license       = "MIT"

src/private/jni_api.nim

@@ -137,13 +137,13 @@ proc newJavaException*(ex: JVMObject): ref JavaException =
 proc newJVMObject*(o: jobject): JVMObject
 proc newJVMObjectConsumingLocalRef*(o: jobject): JVMObject
 
-template checkException: stmt =
+template checkException() =
   if theEnv != nil and theEnv.ExceptionCheck(theEnv) == JVM_TRUE:
     let ex = theEnv.ExceptionOccurred(theEnv).newJVMObjectConsumingLocalRef
     theEnv.ExceptionClear(theEnv)
     raise newJavaException(ex)
 
-macro callVM*(s: expr): expr =
+macro callVM*(s: untyped): untyped =
   result = quote do:
     let res = `s`
     checkException()
@@ -187,6 +187,12 @@ proc getByName*(T: typedesc[JVMClass], name: string): JVMClass =
   ## Finds class by it's name (not fqcn)
   T.getByFqcn(name.fqcn)
 
+proc getJVMClass(o: jobject): JVMClass {.inline.} =
+  checkInit
+  let c = callVM theEnv.GetObjectClass(theEnv, o)
+  result = c.newJVMClass
+  theEnv.deleteLocalRef(c)
+
 proc get*(c: JVMClass): jclass =
   c.cls
 
@@ -305,11 +311,8 @@ proc toJValue*(o: JVMObject): jvalue =
   o.get.toJValue
 
 proc getJVMClass*(o: JVMObject): JVMClass =
-  checkInit
   assert(o.get != nil)
-  let c = callVM theEnv.GetObjectClass(theEnv, o.get)
-  result = c.newJVMClass
-  theEnv.deleteLocalRef(c)
+  getJVMClass(o.get)
 
 proc equalsRaw*(v1, v2: JVMObject): jboolean =
   # This is low level ``equals`` version
@@ -324,26 +327,38 @@ proc equalsRaw*(v1, v2: JVMObject): jboolean =
   result = theEnv.CallBooleanMethodA(theEnv, v1.obj, mthId, addr v2w)
 
 proc jstringToStringAux(s: jstring): string =
+  assert(not s.isNil)
   let sz = theEnv.GetStringUTFLength(theEnv, s)
   result = newString(sz)
   theEnv.GetStringUTFRegion(theEnv, s, 0, sz, addr result[0])
 
-proc toStringRaw(o: JVMObject): string =
-  # This is low level ``toString`` version
-  if o.isNil:
-    return nil
-  let cls = theEnv.GetObjectClass(theEnv, o.obj)
+proc toStringRaw(o: jobject): string =
+  # This is low level ``toString`` version.
+  assert(not o.isNil)
+  let cls = theEnv.GetObjectClass(theEnv, o)
   jniAssertEx(cls.pointer != nil, "Can't find object's class")
   const sig = "()" & string.jniSig
   let mthId = theEnv.GetMethodID(theEnv, cls, "toString", sig)
   theEnv.deleteLocalRef(cls)
   jniAssertEx(mthId != nil, "Can't find ``toString`` method")
-  let s = theEnv.CallObjectMethodA(theEnv, o.obj, mthId, nil).jstring
+  let s = theEnv.CallObjectMethodA(theEnv, o, mthId, nil).jstring
   if s == nil:
     return nil
   result = jstringToStringAux(s)
   theEnv.deleteLocalRef(s)
 
+proc toStringRawConsumingLocalRef(o: jobject): string =
+  # This is low level ``toString`` version
+  if not o.isNil:
+    result = toStringRaw(o)
+    theEnv.deleteLocalRef(o)
+
+proc toStringRaw(o: JVMObject): string =
+  # This is low level ``toString`` version
+  if o.isNil:
+    return nil
+  toStringRaw(o.obj)
+
 proc callVoidMethod*(o: JVMObject, id: JVMMethodID, args: openarray[jvalue] = []) =
   checkInit
   let a = if args.len == 0: nil else: unsafeAddr args[0]
@@ -359,7 +374,7 @@ proc callVoidMethod*(o: JVMObject, name, sig: cstring, args: openarray[jvalue] =
 ####################################################################################################
 # Arrays support
 
-template genArrayType(typ, arrTyp: typedesc, typName: untyped): stmt {.immediate.} =
+template genArrayType(typ, arrTyp: typedesc, typName: untyped): untyped =
 
   # Creation
 
@@ -510,7 +525,7 @@ genArrayType(JVMObject, jobjectArray, Object)
 ####################################################################################################
 # Fields accessors generation
 
-template genField(typ: typedesc, typName: untyped): stmt =
+template genField(typ: typedesc, typName: untyped): untyped =
   proc `get typName`*(c: JVMClass, id: JVMFieldID): `typ` =
     checkInit
     when `typ` is JVMObject:
@@ -616,7 +631,7 @@ genField(jboolean, Boolean)
 ####################################################################################################
 # Methods generation
 
-template genMethod(typ: typedesc, typName: untyped): stmt =
+template genMethod(typ: typedesc, typName: untyped): untyped =
   proc `call typName Method`*(c: JVMClass, id: JVMMethodID, args: openarray[jvalue] = []): `typ` =
     checkInit
     let a = if args.len == 0: nil else: unsafeAddr args[0]
@@ -713,7 +728,7 @@ template jarrayToSeqImpl[T](arr: jarray, res: var seq[T]) =
       res[i] = T.fromJObjectConsumingLocalRef(theEnv.GetObjectArrayElement(theEnv, arr.jobjectArray, i.jsize))
   elif T is string:
     for i in 0..<res.len:
-      res[i] = theEnv.GetObjectArrayElement(theEnv, arr.jobjectArray, i.jsize).newJVMObjectConsumingLocalRef.toStringRaw
+      res[i] = toStringRawConsumingLocalRef(theEnv.GetObjectArrayElement(theEnv, arr.jobjectArray, i.jsize))
   else:
     {.fatal: "Sequences is not supported for the supplied type".}
 
@@ -727,7 +742,7 @@ template getPropValue*(T: typedesc, o: untyped, id: JVMFieldID): untyped =
   elif T is JPrimitiveType:
     T.getProp(o, id)
   elif T is string:
-    JVMObject.getPropRaw(o, id).newJVMObjectConsumingLocalRef.toStringRaw
+    toStringRawConsumingLocalRef(JVMObject.getPropRaw(o, id))
   elif T is JVMObject:
     T.fromJObjectConsumingLocalRef(JVMObject.getPropRaw(o, id))
   elif T is seq:
@@ -769,7 +784,7 @@ template callMethod*(T: typedesc, o: untyped, methodId: JVMMethodID, args: opena
   elif T is seq:
     T(jarrayToSeqConsumingLocalRef(o.callObjectMethodRaw(methodId, args).jarray, T))
   elif T is string:
-    o.callObjectMethod(methodId, args).toStringRaw
+    toStringRawConsumingLocalRef(o.callObjectMethodRaw(methodId, args))
   elif T is JVMObject:
     T.fromJObjectConsumingLocalRef(o.callObjectMethodRaw(methodId, args))
   else:

src/private/jni_generator.nim

@@ -233,7 +233,7 @@ proc fillProcDef(n: NimNode, def: NimNode): NimNode {.compileTime.} =
       `def`.genericTypes.add(`v`)
     result.add(q)
 
-macro parseProcDefTest*(i: untyped, s: expr): stmt =
+macro parseProcDefTest*(i: untyped, s: untyped): untyped =
   result = fillProcDef(s[0], i)
 
 ####################################################################################################
@@ -320,13 +320,13 @@ proc fillClassDef(c: NimNode, def: NimNode): NimNode {.compileTime.} =
       `def`.parentGenericTypes.add(`v`)
     result.add(q)
 
-macro parseClassDefTest*(i: untyped, s: expr): stmt =
+macro parseClassDefTest*(i: untyped, s: untyped): untyped =
   result = fillClassDef(if s.kind == nnkStmtList: s[0] else: s, i)
 
 ####################################################################################################
 # Type generator
 
-template identEx(isExported: bool, name: string, isSetter = false, isQuoted = false): expr =
+template identEx(isExported: bool, name: string, isSetter = false, isQuoted = false): untyped =
   let id =
     if isSetter:
       newNimNode(nnkAccQuoted).add(ident(name), ident("="))
@@ -385,17 +385,17 @@ proc generateClassDef(cd: ClassDef): NimNode {.compileTime.} =
   let seqEqOpIdent = identEx(cd.isExported, "==", isQuoted = true)
   result = quote do:
     type `classNameEx` = ref object of `parentType`
-    proc `jniSigIdent`(t: typedesc[`className`]): string = sigForClass(`jName`)
-    proc `jniSigIdent`(t: typedesc[openarray[`className`]]): string = "[" & sigForClass(`jName`)
-    proc `getClassId`(t: typedesc[`className`]): JVMClass =
+    proc `jniSigIdent`(t: typedesc[`className`]): string {.used, inline.} = sigForClass(`jName`)
+    proc `jniSigIdent`(t: typedesc[openarray[`className`]]): string {.used, inline.} = "[" & sigForClass(`jName`)
+    proc `getClassId`(t: typedesc[`className`]): JVMClass {.used, inline.} =
       JVMClass.getByFqcn(toConstCString(fqcn(`jName`)))
-    proc toJVMObject(v: `className`): JVMObject =
+    proc toJVMObject(v: `className`): JVMObject {.used, inline.} =
       v.JVMObject
-    proc toJValue(v: `className`): jvalue =
+    proc toJValue(v: `className`): jvalue {.used, inline.} =
       v.JVMObject.get.toJValue
-    proc `eqOpIdent`(v1, v2: `className`): bool =
+    proc `eqOpIdent`(v1, v2: `className`): bool {.used, inline.} =
       return (v1.equalsRaw(v2) == JVM_TRUE)
-    proc `seqEqOpIdent`(v1, v2: seq[`className`]): bool =
+    proc `seqEqOpIdent`(v1, v2: seq[`className`]): bool {.used.} =
       if v1.len != v2.len:
         return false
       else:
@@ -451,34 +451,45 @@ proc generateMethod(cd: ClassDef, pd: ProcDef, def: NimNode): NimNode =
   assert(not (pd.isConstructor or pd.isProp))
 
   let sig = getProcSignature(cd, pd)
-  let cname = cd.jName.newStrLitNode
   let pname = pd.jName.newStrLitNode
   let ctype = cd.name.ident
   result = def.copyNimTree
   fillGenericParameters(cd, pd, result)
   result.pragma = newEmptyNode()
+
   var objToCall: NimNode
+  var objToCallIdent: NimNode
+  var mIdIdent = ident"mId"
+
   # Add first parameter
   if pd.isStatic:
     result.params.insert(1, newIdentDefs(ident"theClassType", cd.mkTypedesc))
+    objToCallIdent = ident"clazz"
     objToCall = quote do:
-      `ctype`.getJVMClassForType
+      let `objToCallIdent` = `ctype`.getJVMClassForType
+      let `mIdIdent` = `objToCallIdent`.getStaticMethodId(`pname`, toConstCString(`sig`))
+
   else:
-    result.params.insert(1, newIdentDefs(ident"this", cd.mkType))
-    objToCall = ident"this"
+    objToCallIdent = ident"this"
+    result.params.insert(1, newIdentDefs(objToCallIdent, cd.mkType))
+    objToCall = quote do:
+      let `mIdIdent` = `objToCallIdent`.getMethodId(`pname`, toConstCString(`sig`))
+
   let retType = parseExpr(pd.retType)
-  var mId: NimNode
-  if pd.isStatic:
-    mId = quote do:
-      `objToCall`.getStaticMethodId(`pname`, toConstCString(`sig`))
-  else:
-    mId = quote do:
-      `objToCall`.getMethodId(`pname`, toConstCString(`sig`))
   let ai = ident"args"
   let args = generateArgs(pd, ai)
   result.body = quote do:
+    # Disabling GC is a must on Android (and maybe other platforms) in release
+    # mode. Otherwise Nim GC may kick in and finalize the JVMObject we're passing
+    # to JNI call before the actual JNI call is made. That is likely caused
+    # by release optimizations that prevent Nim GC from "seeing" the JVMObjects
+    # on the stack after their last usage, even though from the code POV they
+    # are still here.
+    GC_disable()
+    `objToCall`
     `args`
-    callMethod(`retType`, `objToCall`, `mId`, `ai`)
+    GC_enable()
+    callMethod(`retType`, `objToCallIdent`, `mIdIdent`, `ai`)
 
 proc generateProperty(cd: ClassDef, pd: ProcDef, def: NimNode, isSetter: bool): NimNode =
   assert pd.isProp
@@ -538,18 +549,18 @@ proc generateClassImpl(cd: ClassDef, body: NimNode): NimNode {.compileTime.} =
       result.add generateProc(cd, def)
   else: result.add generateProc(cd, body)
 
-macro jclass*(head: expr, body: expr): stmt {.immediate.} =
+macro jclass*(head: untyped, body: untyped): untyped =
   result = newStmtList()
   let cd = parseClassDef(head)
   result.add generateClassDef(cd)
   result.add generateClassImpl(cd, body)
 
-macro jclassDef*(head: expr): stmt {.immediate.} =
+macro jclassDef*(head: untyped): untyped =
   result = newStmtList()
   let cd = parseClassDef(head)
   result.add generateClassDef(cd)
 
-macro jclassImpl*(head: expr, body: expr): stmt {.immediate.} =
+macro jclassImpl*(head: untyped, body: untyped): untyped =
   result = newStmtList()
   let cd = parseClassDef(head)
   result.add generateClassImpl(cd, body)