fix exception leak, finalizers are broken (#5)

nim-lang/Nim#4851
status-im · Dec 25, 2020 · 8994b67 · 8994b67
1 parent f147329
commit 8994b67
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 16 deletions.
diff --git a/examples/miniupnpc_test.nim b/examples/miniupnpc_test.nim
@@ -8,7 +8,7 @@
 
 # This is the equivalent of miniupnpc/pymoduletest.py (without the command line args).
 
-import nat_traversal/miniupnpc, result, strformat
+import nat_traversal/miniupnpc, strformat
 
 template checkError(expr, body: untyped): untyped =
   block:

diff --git a/examples/natpmp_test.nim b/examples/natpmp_test.nim
@@ -6,7 +6,7 @@
 # This file may not be copied, modified, or distributed except according to
 # those terms.
 
-import nat_traversal/natpmp, result, strformat
+import nat_traversal/natpmp, strformat
 
 template checkError(expr, body: untyped): untyped =
   block:

diff --git a/nat_traversal/miniupnpc.nim b/nat_traversal/miniupnpc.nim
@@ -10,6 +10,8 @@
 # headers and library location #
 ################################
 
+{.push raises: [Defect].}
+
 import ./utils
 
 when defined(miniupnpcUseSystemLibs):
@@ -515,7 +517,8 @@ proc UPNPIGD_IsConnected*(a1: ptr UPNPUrls; a2: ptr IGDdatas): cint {.
 # custom wrappers #
 ###################
 
-import stew/result, strutils
+import stew/results, std/strutils
+export results
 
 type Miniupnp* = ref object
   devList*: ptr UPNPDev
@@ -530,13 +533,14 @@ type Miniupnp* = ref object
   error*: cint
   lanAddr*: string
 
-proc miniupnpFinalizer(x: Miniupnp) =
-  freeUPNPDevlist(x.devList)
-  x.devList = nil
-  freeUPNPUrls(addr(x.urls))
+proc close*(x: Miniupnp) =
+  if x.devList != nil:
+    freeUPNPDevlist(x.devList)
+    x.devList = nil
+    freeUPNPUrls(addr(x.urls))
 
 proc newMiniupnp*(): Miniupnp =
-  new(result, miniupnpFinalizer)
+  new(result)
   result.ttl = 2.cuchar
 
 proc `=deepCopy`*(x: Miniupnp): Miniupnp =
@@ -812,9 +816,16 @@ proc getSpecificPortMapping*(self: Miniupnp,
     trimString(portMapping.internalPort)
     trimString(portMapping.description)
     trimString(enabledStr)
-    portMapping.enabled = bool(parseInt(enabledStr))
+    portMapping.enabled = try:
+        bool(parseInt(enabledStr))
+      except ValueError: # shouldn't happen..
+        false
     trimString(leaseDurationStr)
-    portMapping.leaseDuration = parseBiggestUInt(leaseDurationStr)
+    portMapping.leaseDuration =
+      try:
+        parseBiggestUInt(leaseDurationStr)
+      except ValueError:
+        return err("upnp: cannot parsse lease duration")
     result.ok(portMapping)
   else:
     result.err(upnpError(res))
@@ -848,13 +859,25 @@ proc getGenericPortMapping*(self: Miniupnp,
     trimString(portMapping.internalClient)
     trimString(portMapping.internalPort)
     trimString(protocolStr)
-    portMapping.protocol = parseEnum[UPNPProtocol](protocolStr)
+    portMapping.protocol =
+      try:
+        parseEnum[UPNPProtocol](protocolStr)
+      except ValueError:
+        return err("upnp: cannot parse upnp protocol")
     trimString(portMapping.description)
     trimString(enabledStr)
-    portMapping.enabled = bool(parseInt(enabledStr))
+    portMapping.enabled =
+      try:
+        bool(parseInt(enabledStr))
+      except ValueError:
+        false
     trimString(portMapping.remoteHost)
     trimString(leaseDurationStr)
-    portMapping.leaseDuration = parseBiggestUInt(leaseDurationStr)
+    portMapping.leaseDuration =
+      try:
+        parseBiggestUInt(leaseDurationStr)
+      except ValueError:
+        return err("upnp: cannot parse duration")
     result.ok(portMapping)
   else:
     result.err(upnpError(res))

diff --git a/nat_traversal/natpmp.nim b/nat_traversal/natpmp.nim
@@ -10,6 +10,8 @@
 # headers and library location #
 ################################
 
+{.push raises: [Defect].}
+
 import os
 when defined(windows):
   import winlean
@@ -194,16 +196,18 @@ proc strnatpmperr*(t: cint): cstring {.importc: "strnatpmperr", header: "natpmp.
 # custom wrappers #
 ###################
 
-import stew/results
+import
+  stew/results
+export results
 
 type NatPmp* {.packed.} = ref object
   cstruct*: natpmp_t
 
-proc natpmpFinalizer(x: NatPmp) =
+proc close*(x: NatPmp) =
   discard closenatpmp(addr(x.cstruct))
 
 proc newNatPmp*(): NatPmp =
-  new(result, natpmpFinalizer)
+  new(result)
 
 proc init*(self: NatPmp): Result[bool, cstring] =
   let res = initnatpmp(addr(self.cstruct), 0, 0)