From 3bc0bc36f7c2ef655c5ecf5fa884bef07286328f Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Thu, 21 Oct 2021 17:22:11 +0300 Subject: [PATCH] Change Future identifier type from `int` to `uint`. (#228) * Change Future identifier type from `int` to `uint64`. * Fix compilation error and tests. * Add integer overflow test, to avoid confusion with next Nim versions, we expect that unsigned integers do not raise any exceptions if overflow happens. * Switch from `uint64` to `uint` type. * Add `uint` overflow tests. --- chronos/asyncfutures2.nim | 13 ++++++----- chronos/asyncloop.nim | 2 +- chronos/debugutils.nim | 20 ++++++++++------ tests/testfut.nim | 37 ++++++++++++++++++++++++++++- tests/testutils.nim | 49 ++++++++++++++++++++------------------- 5 files changed, 82 insertions(+), 39 deletions(-) diff --git a/chronos/asyncfutures2.nim b/chronos/asyncfutures2.nim index 2e05dcb85..2b2311bef 100644 --- a/chronos/asyncfutures2.nim +++ b/chronos/asyncfutures2.nim @@ -9,6 +9,7 @@ # MIT license (LICENSE-MIT) import std/[os, tables, strutils, heapqueue, options, deques, sequtils] +import stew/base10 import ./srcloc export srcloc @@ -38,7 +39,7 @@ type state*: FutureState error*: ref CatchableError ## Stored exception mustCancel*: bool - id*: int + id*: uint64 when defined(chronosStackTrace): errorStackTrace*: StackTrace @@ -75,10 +76,10 @@ type FutureList* = object head*: FutureBase tail*: FutureBase - count*: int + count*: uint -var currentID* {.threadvar.}: int -currentID = 0 +var currentID* {.threadvar.}: uint64 +currentID = 0'u64 when defined(chronosFutureTracking): var futureList* {.threadvar.}: FutureList @@ -86,12 +87,12 @@ when defined(chronosFutureTracking): template setupFutureBase(loc: ptr SrcLoc) = new(result) + currentID.inc() result.state = FutureState.Pending when defined(chronosStackTrace): result.stackTrace = getStackTrace() result.id = currentID result.location[LocCreateIndex] = loc - currentID.inc() when defined(chronosFutureTracking): result.next = nil @@ -198,7 +199,7 @@ proc checkFinished(future: FutureBase, loc: ptr SrcLoc) = var msg = "" msg.add("An attempt was made to complete a Future more than once. ") msg.add("Details:") - msg.add("\n Future ID: " & $future.id) + msg.add("\n Future ID: " & Base10.toString(future.id)) msg.add("\n Creation location:") msg.add("\n " & $future.location[LocCreateIndex]) msg.add("\n First completion location:") diff --git a/chronos/asyncloop.nim b/chronos/asyncloop.nim index bfa7f591c..03d1c96c7 100644 --- a/chronos/asyncloop.nim +++ b/chronos/asyncloop.nim @@ -1112,7 +1112,7 @@ when defined(chronosFutureTracking): yield slider slider = slider.next - proc pendingFuturesCount*(): int = + proc pendingFuturesCount*(): uint = ## Returns number of pending Futures (Future[T] objects which not yet ## completed, cancelled or failed). futureList.count diff --git a/chronos/debugutils.nim b/chronos/debugutils.nim index 207b71259..c893f66f8 100644 --- a/chronos/debugutils.nim +++ b/chronos/debugutils.nim @@ -12,6 +12,9 @@ import ./asyncloop export asyncloop +when defined(chronosFutureTracking): + import stew/base10 + const AllFutureStates* = {FutureState.Pending, FutureState.Cancelled, FutureState.Finished, FutureState.Failed} @@ -28,7 +31,7 @@ proc dumpPendingFutures*(filter = AllFutureStates): string = ## not yet finished). ## 2. Future[T] objects with ``FutureState.Finished/Cancelled/Failed`` state ## which callbacks are scheduled, but not yet fully processed. - var count = 0 + var count = 0'u var res = "" when defined(chronosFutureTracking): for item in pendingFutures(): @@ -41,13 +44,16 @@ proc dumpPendingFutures*(filter = AllFutureStates): string = "\"unspecified\"" else: "\"" & procedure & "\"" - let item = "Future[" & $item.id & "] with name " & $procname & - " created at " & "<" & filename & ":" & $loc.line & ">" & + let item = "Future[" & Base10.toString(item.id) & "] with name " & + $procname & " created at " & "<" & filename & ":" & + Base10.toString(uint(loc.line)) & ">" & " and state = " & $item.state & "\n" res.add(item) - result = $count & " pending Future[T] objects found:\n" & $res + Base10.toString(count) & " pending Future[T] objects found:\n" & $res + else: + "0 pending Future[T] objects found\n" -proc pendingFuturesCount*(filter: set[FutureState]): int = +proc pendingFuturesCount*(filter: set[FutureState]): uint = ## Returns number of `pending` Future[T] objects which satisfy the ``filter`` ## condition. ## @@ -57,10 +63,10 @@ proc pendingFuturesCount*(filter: set[FutureState]): int = if filter == AllFutureStates: pendingFuturesCount() else: - var res = 0 + var res = 0'u for item in pendingFutures(): if item.state in filter: inc(res) res else: - 0 + 0'u diff --git a/tests/testfut.nim b/tests/testfut.nim index 7f9772615..656cacd69 100644 --- a/tests/testfut.nim +++ b/tests/testfut.nim @@ -1315,4 +1315,39 @@ suite "Future[T] behavior test suite": f2.finished() f3.finished() - + test "Unsigned integer overflow test": + check: + 0xFFFF_FFFF_FFFF_FFFF'u64 + 1'u64 == 0'u64 + 0xFFFF_FFFF'u32 + 1'u32 == 0'u32 + + when sizeof(uint) == 8: + check 0xFFFF_FFFF_FFFF_FFFF'u + 1'u == 0'u + else: + check 0xFFFF_FFFF'u + 1'u == 0'u + + var v1_64 = 0xFFFF_FFFF_FFFF_FFFF'u64 + var v2_64 = 0xFFFF_FFFF_FFFF_FFFF'u64 + var v1_32 = 0xFFFF_FFFF'u32 + var v2_32 = 0xFFFF_FFFF'u32 + inc(v1_64) + inc(v1_32) + check: + v1_64 == 0'u64 + v2_64 + 1'u64 == 0'u64 + v1_32 == 0'u32 + v2_32 + 1'u32 == 0'u32 + + when sizeof(uint) == 8: + var v1_u = 0xFFFF_FFFF_FFFF_FFFF'u + var v2_u = 0xFFFF_FFFF_FFFF_FFFF'u + inc(v1_u) + check: + v1_u == 0'u + v2_u + 1'u == 0'u + else: + var v1_u = 0xFFFF_FFFF'u + var v2_u = 0xFFFF_FFFF'u + inc(v1_u) + check: + v1_u == 0'u + v2_u + 1'u == 0'u diff --git a/tests/testutils.nim b/tests/testutils.nim index 79e55ec7e..3e01f78a9 100644 --- a/tests/testutils.nim +++ b/tests/testutils.nim @@ -12,19 +12,20 @@ when defined(nimHasUsed): {.used.} suite "Asynchronous utilities test suite": when defined(chronosFutureTracking): - proc getCount(): int = + proc getCount(): uint = # This procedure counts number of Future[T] in double-linked list via list # iteration. - result = 0 + var res = 0'u for item in pendingFutures(): - inc(result) + inc(res) + res test "Future clean and leaks test": when defined(chronosFutureTracking): - if pendingFuturesCount(WithoutFinished) == 0: - if pendingFuturesCount(OnlyFinished) > 0: + if pendingFuturesCount(WithoutFinished) == 0'u: + if pendingFuturesCount(OnlyFinished) > 0'u: poll() - check pendingFuturesCount() == 0 + check pendingFuturesCount() == 0'u else: echo dumpPendingFutures() check false @@ -35,31 +36,31 @@ suite "Asynchronous utilities test suite": when defined(chronosFutureTracking): var fut1 = newFuture[void]() check: - getCount() == 1 - pendingFuturesCount() == 1 + getCount() == 1'u + pendingFuturesCount() == 1'u var fut2 = newFuture[void]() check: - getCount() == 2 - pendingFuturesCount() == 2 + getCount() == 2'u + pendingFuturesCount() == 2'u var fut3 = newFuture[void]() check: - getCount() == 3 - pendingFuturesCount() == 3 + getCount() == 3'u + pendingFuturesCount() == 3'u fut1.complete() poll() check: - getCount() == 2 - pendingFuturesCount() == 2 + getCount() == 2'u + pendingFuturesCount() == 2'u fut2.fail(newException(ValueError, "")) poll() check: - getCount() == 1 - pendingFuturesCount() == 1 + getCount() == 1'u + pendingFuturesCount() == 1'u fut3.cancel() poll() check: - getCount() == 0 - pendingFuturesCount() == 0 + getCount() == 0'u64 + pendingFuturesCount() == 0'u64 else: skip() @@ -70,17 +71,17 @@ suite "Asynchronous utilities test suite": var fut = simpleProc() check: - getCount() == 2 - pendingFuturesCount() == 2 + getCount() == 2'u64 + pendingFuturesCount() == 2'u64 waitFor fut check: - getCount() == 1 - pendingFuturesCount() == 1 + getCount() == 1'u64 + pendingFuturesCount() == 1'u64 poll() check: - getCount() == 0 - pendingFuturesCount() == 0 + getCount() == 0'u64 + pendingFuturesCount() == 0'u64 else: skip()