From 44cada9c551f1ac5d2a9c6cd2dbdfc8b112b4816 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sun, 3 Mar 2024 18:13:37 +0100 Subject: [PATCH] use new Chronos `trackCounter` APIs for leaks checks in tests (#1038) --- libp2p/stream/lpstream.nim | 30 +++----------------------- libp2p/transports/tcptransport.nim | 34 +++--------------------------- tests/helpers.nim | 24 ++++++++------------- tests/testbufferstream.nim | 5 ++--- 4 files changed, 17 insertions(+), 76 deletions(-) diff --git a/libp2p/stream/lpstream.nim b/libp2p/stream/lpstream.nim index 5701b1693..49446d589 100644 --- a/libp2p/stream/lpstream.nim +++ b/libp2p/stream/lpstream.nim @@ -1,5 +1,5 @@ # Nim-LibP2P -# Copyright (c) 2023 Status Research & Development GmbH +# Copyright (c) 2023-2024 Status Research & Development GmbH # Licensed under either of # * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE)) # * MIT license ([LICENSE-MIT](LICENSE-MIT)) @@ -77,30 +77,6 @@ type opened*: uint64 closed*: uint64 -proc setupStreamTracker*(name: string): StreamTracker = - let tracker = new StreamTracker - - proc dumpTracking(): string {.gcsafe.} = - return "Opened " & tracker.id & ": " & $tracker.opened & "\n" & - "Closed " & tracker.id & ": " & $tracker.closed - - proc leakTransport(): bool {.gcsafe.} = - return (tracker.opened != tracker.closed) - - tracker.id = name - tracker.opened = 0 - tracker.closed = 0 - tracker.dump = dumpTracking - tracker.isLeaked = leakTransport - addTracker(name, tracker) - - return tracker - -proc getStreamTracker(name: string): StreamTracker {.gcsafe.} = - result = cast[StreamTracker](getTracker(name)) - if isNil(result): - result = setupStreamTracker(name) - proc newLPStreamReadError*(p: ref CatchableError): ref LPStreamReadError = var w = newException(LPStreamReadError, "Read stream failed") w.msg = w.msg & ", originated from [" & $p.name & "] " & p.msg @@ -157,7 +133,7 @@ method initStream*(s: LPStream) {.base.} = s.oid = genOid() libp2p_open_streams.inc(labelValues = [s.objName, $s.dir]) - inc getStreamTracker(s.objName).opened + trackCounter(s.objName) trace "Stream created", s, objName = s.objName, dir = $s.dir proc join*( @@ -304,7 +280,7 @@ method closeImpl*(s: LPStream): Future[void] {.async, base.} = ## Implementation of close - called only once trace "Closing stream", s, objName = s.objName, dir = $s.dir libp2p_open_streams.dec(labelValues = [s.objName, $s.dir]) - inc getStreamTracker(s.objName).closed + untrackCounter(s.objName) s.closeEvent.fire() trace "Closed stream", s, objName = s.objName, dir = $s.dir diff --git a/libp2p/transports/tcptransport.nim b/libp2p/transports/tcptransport.nim index cc60f3f94..531c8e517 100644 --- a/libp2p/transports/tcptransport.nim +++ b/libp2p/transports/tcptransport.nim @@ -1,5 +1,5 @@ # Nim-LibP2P -# Copyright (c) 2023 Status Research & Development GmbH +# Copyright (c) 2023-2024 Status Research & Development GmbH # Licensed under either of # * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE)) # * MIT license ([LICENSE-MIT](LICENSE-MIT)) @@ -42,36 +42,8 @@ type acceptFuts: seq[Future[StreamTransport]] connectionsTimeout: Duration - TcpTransportTracker* = ref object of TrackerBase - opened*: uint64 - closed*: uint64 - TcpTransportError* = object of transport.TransportError -proc setupTcpTransportTracker(): TcpTransportTracker {.gcsafe, raises: [].} - -proc getTcpTransportTracker(): TcpTransportTracker {.gcsafe.} = - result = cast[TcpTransportTracker](getTracker(TcpTransportTrackerName)) - if isNil(result): - result = setupTcpTransportTracker() - -proc dumpTracking(): string {.gcsafe.} = - var tracker = getTcpTransportTracker() - result = "Opened tcp transports: " & $tracker.opened & "\n" & - "Closed tcp transports: " & $tracker.closed - -proc leakTransport(): bool {.gcsafe.} = - var tracker = getTcpTransportTracker() - result = (tracker.opened != tracker.closed) - -proc setupTcpTransportTracker(): TcpTransportTracker = - result = new TcpTransportTracker - result.opened = 0 - result.closed = 0 - result.dump = dumpTracking - result.isLeaked = leakTransport - addTracker(TcpTransportTrackerName, result) - proc connHandler*(self: TcpTransport, client: StreamTransport, observedAddr: Opt[MultiAddress], @@ -163,7 +135,7 @@ method start*( await procCall Transport(self).start(addrs) trace "Starting TCP transport" - inc getTcpTransportTracker().opened + trackCounter(TcpTransportTrackerName) for i, ma in addrs: if not self.handles(ma): @@ -217,7 +189,7 @@ method stop*(self: TcpTransport) {.async.} = self.servers = @[] trace "Transport stopped" - inc getTcpTransportTracker().closed + untrackCounter(TcpTransportTrackerName) except CatchableError as exc: trace "Error shutting down tcp transport", exc = exc.msg diff --git a/tests/helpers.nim b/tests/helpers.nim index f18a720b7..2f3cd8835 100644 --- a/tests/helpers.nim +++ b/tests/helpers.nim @@ -35,25 +35,19 @@ const ChronosStreamTrackerName ] -iterator testTrackers*(extras: openArray[string] = []): TrackerBase = - for name in trackerNames: - let t = getTracker(name) - if not isNil(t): yield t - for name in extras: - let t = getTracker(name) - if not isNil(t): yield t - template checkTracker*(name: string) = - var tracker = getTracker(name) - if tracker.isLeaked(): - checkpoint tracker.dump() + if isCounterLeaked(name): + let + tracker = getTrackerCounter(name) + trackerDescription = + "Opened " & name & ": " & $tracker.opened & "\n" & + "Closed " & name & ": " & $tracker.closed + checkpoint trackerDescription fail() template checkTrackers*() = - for tracker in testTrackers(): - if tracker.isLeaked(): - checkpoint tracker.dump() - fail() + for name in trackerNames: + checkTracker(name) # Also test the GC is not fooling with us when defined(nimHasWarnBareExcept): {.push warning[BareExcept]:off.} diff --git a/tests/testbufferstream.nim b/tests/testbufferstream.nim index 905459de8..8a9f8b5f2 100644 --- a/tests/testbufferstream.nim +++ b/tests/testbufferstream.nim @@ -1,7 +1,7 @@ {.used.} # Nim-Libp2p -# Copyright (c) 2023 Status Research & Development GmbH +# Copyright (c) 2023-2024 Status Research & Development GmbH # Licensed under either of # * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE)) # * MIT license ([LICENSE-MIT](LICENSE-MIT)) @@ -18,8 +18,7 @@ import ./helpers suite "BufferStream": teardown: - # echo getTracker(BufferStreamTrackerName).dump() - check getTracker(BufferStreamTrackerName).isLeaked() == false + checkTrackers() asyncTest "push data to buffer": let buff = BufferStream.new()