From 229de5f842dd38a42a55d7dc4cedf38b5d034b63 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Fri, 31 Mar 2023 07:35:04 +0200 Subject: [PATCH] Compile-time configuration (#371) This PR moves all compile-time configuration to a single module, simplifying documentation and access to these features. Upcomfing features may be enabled either individually, or through a new `chronosPreviewV4` catch-all designed to allow code to be prepared for increased strictness in future chronos releases. `-d:chronosDebug` may be used to enable the existing debugging helpers together. --- README.md | 6 +++++ chronos.nimble | 7 ++--- chronos/asyncfutures2.nim | 42 ++++++++++++++++------------- chronos/asyncloop.nim | 6 ++--- chronos/asyncmacro2.nim | 8 +++--- chronos/config.nim | 57 +++++++++++++++++++++++++++++++++++++++ chronos/debugutils.nim | 8 +++--- tests/testaddress.nim | 2 +- tests/testasyncstream.nim | 2 +- tests/testbugs.nim | 2 +- tests/testdatagram.nim | 2 +- tests/testfut.nim | 2 +- tests/testhttpclient.nim | 2 +- tests/testhttpserver.nim | 2 +- tests/testmacro.nim | 2 +- tests/testnet.nim | 2 +- tests/testratelimit.nim | 4 ++- tests/testserver.nim | 2 +- tests/testshttpserver.nim | 2 +- tests/testsignal.nim | 2 +- tests/testsoon.nim | 2 +- tests/teststream.nim | 2 +- tests/testsync.nim | 2 +- tests/testtime.nim | 2 +- tests/testutils.nim | 12 ++++----- 25 files changed, 126 insertions(+), 56 deletions(-) create mode 100644 chronos/config.nim diff --git a/README.md b/README.md index c63d254..c0cc230 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,12 @@ Known `async` backends include: ``none`` can be used when a library supports both a synchronous and asynchronous API, to disable the latter. +### Compile-time configuration + +`chronos` contains several compile-time [configuration options](./chronos/config.nim) enabling stricter compile-time checks and debugging helpers whose runtime cost may be significant. + +Strictness options generally will become default in future chronos releases and allow adapting existing code without changing the new version - see the [`config.nim`](./chronos/config.nim) module for more information. + ## TODO * Pipe/Subprocess Transports. * Multithreading Stream/Datagram servers diff --git a/chronos.nimble b/chronos.nimble index 18f7a47..dfb343c 100644 --- a/chronos.nimble +++ b/chronos.nimble @@ -32,10 +32,11 @@ proc run(args, path: string) = task test, "Run all tests": for args in [ - "-d:useSysAssert -d:useGcAssert", - "-d:chronosStackTrace -d:chronosStrictException", + "-d:debug -d:chronosDebug", + "-d:debug -d:chronosPreviewV4", + "-d:debug -d:chronosDebug -d:useSysAssert -d:useGcAssert", "-d:release", - "-d:release -d:chronosFutureTracking", + "-d:release -d:chronosPreviewV4", ]: run args, "tests/testall" task test_libbacktrace, "test with libbacktrace": diff --git a/chronos/asyncfutures2.nim b/chronos/asyncfutures2.nim index 249bb98..bac4ba1 100644 --- a/chronos/asyncfutures2.nim +++ b/chronos/asyncfutures2.nim @@ -10,7 +10,7 @@ import std/sequtils import stew/base10 -import ./srcloc +import "."/[config, srcloc] export srcloc when defined(nimHasStacktracesModule): @@ -24,7 +24,7 @@ const LocCreateIndex* = 0 LocCompleteIndex* = 1 -when defined(chronosStackTrace): +when chronosStackTrace: type StackTrace = string type @@ -41,11 +41,11 @@ type mustCancel*: bool id*: uint - when defined(chronosStackTrace): + when chronosStackTrace: errorStackTrace*: StackTrace stackTrace: StackTrace ## For debugging purposes only. - when defined(chronosFutureTracking): + when chronosFutureTracking: next*: FutureBase prev*: FutureBase @@ -54,7 +54,7 @@ type # How much refactoring is needed to make this a regular non-ref type? # Obviously, it will still be allocated on the heap when necessary. Future*[T] = ref object of FutureBase ## Typed future. - when defined(chronosStrictException): + when chronosStrictException: closure*: iterator(f: Future[T]): FutureBase {.raises: [Defect, CatchableError], gcsafe.} else: closure*: iterator(f: Future[T]): FutureBase {.raises: [Defect, CatchableError, Exception], gcsafe.} @@ -80,23 +80,27 @@ type tail*: FutureBase count*: uint -var currentID* {.threadvar.}: uint -currentID = 0'u +when chronosFutureId: + var currentID* {.threadvar.}: uint +else: + template id*(f: FutureBase): uint = + cast[uint](addr f[]) -when defined(chronosFutureTracking): +when chronosFutureTracking: var futureList* {.threadvar.}: FutureList futureList = FutureList() template setupFutureBase(loc: ptr SrcLoc) = new(result) - currentID.inc() result.state = FutureState.Pending - when defined(chronosStackTrace): + when chronosStackTrace: result.stackTrace = getStackTrace() - result.id = currentID + when chronosFutureId: + currentID.inc() + result.id = currentID result.location[LocCreateIndex] = loc - when defined(chronosFutureTracking): + when chronosFutureTracking: result.next = nil result.prev = futureList.tail if not(isNil(futureList.tail)): @@ -160,7 +164,7 @@ proc done*(future: FutureBase): bool {.inline.} = ## This is an alias for ``completed(future)`` procedure. completed(future) -when defined(chronosFutureTracking): +when chronosFutureTracking: proc futureDestructor(udata: pointer) = ## This procedure will be called when Future[T] got finished, cancelled or ## failed and all Future[T].callbacks are already scheduled and processed. @@ -188,7 +192,7 @@ proc checkFinished(future: FutureBase, loc: ptr SrcLoc) = msg.add("\n " & $future.location[LocCompleteIndex]) msg.add("\n Second completion location:") msg.add("\n " & $loc) - when defined(chronosStackTrace): + when chronosStackTrace: msg.add("\n Stack trace to moment of creation:") msg.add("\n" & indent(future.stackTrace.strip(), 4)) msg.add("\n Stack trace to moment of secondary completion:") @@ -212,7 +216,7 @@ proc finish(fut: FutureBase, state: FutureState) = item = default(AsyncCallback) # release memory as early as possible fut.callbacks = default(seq[AsyncCallback]) # release seq as well - when defined(chronosFutureTracking): + when chronosFutureTracking: scheduleDestructor(fut) proc complete[T](future: Future[T], val: T, loc: ptr SrcLoc) = @@ -240,7 +244,7 @@ proc fail[T](future: Future[T], error: ref CatchableError, loc: ptr SrcLoc) = if not(future.cancelled()): checkFinished(FutureBase(future), loc) future.error = error - when defined(chronosStackTrace): + when chronosStackTrace: future.errorStackTrace = if getStackTrace(error) == "": getStackTrace() else: @@ -258,7 +262,7 @@ proc cancelAndSchedule(future: FutureBase, loc: ptr SrcLoc) = if not(future.finished()): checkFinished(future, loc) future.error = newCancelledError() - when defined(chronosStackTrace): + when chronosStackTrace: future.errorStackTrace = getStackTrace() future.finish(FutureState.Cancelled) @@ -472,7 +476,7 @@ proc `$`(stackTraceEntries: seq[StackTraceEntry]): string = return exc.msg # Shouldn't actually happen since we set the formatting # string -when defined(chronosStackTrace): +when chronosStackTrace: proc injectStacktrace(future: FutureBase) = const header = "\nAsync traceback:\n" @@ -500,7 +504,7 @@ proc internalCheckComplete*(fut: FutureBase) {. raises: [Defect, CatchableError].} = # For internal use only. Used in asyncmacro if not(isNil(fut.error)): - when defined(chronosStackTrace): + when chronosStackTrace: injectStacktrace(fut) raise fut.error diff --git a/chronos/asyncloop.nim b/chronos/asyncloop.nim index ca16554..abf3edc 100644 --- a/chronos/asyncloop.nim +++ b/chronos/asyncloop.nim @@ -14,9 +14,9 @@ else: {.push raises: [].} from nativesockets import Port -import std/[tables, strutils, heapqueue, lists, options, deques] +import std/[tables, strutils, heapqueue, options, deques] import stew/results -import "."/[osdefs, osutils, timer] +import "."/[config, osdefs, osutils, timer] export Port export timer, results @@ -1263,7 +1263,7 @@ proc getTracker*(id: string): TrackerBase = let loop = getThreadDispatcher() result = loop.trackers.getOrDefault(id, nil) -when defined(chronosFutureTracking): +when chronosFutureTracking: iterator pendingFutures*(): FutureBase = ## Iterates over the list of pending Futures (Future[T] objects which not ## yet completed, cancelled or failed). diff --git a/chronos/asyncmacro2.nim b/chronos/asyncmacro2.nim index 61bf3b5..3fb0bb7 100644 --- a/chronos/asyncmacro2.nim +++ b/chronos/asyncmacro2.nim @@ -193,12 +193,12 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} = # here the possibility of transporting more specific error types here # for example by casting exceptions coming out of `await`.. let raises = nnkBracket.newTree() - when not defined(chronosStrictException): - raises.add(newIdentNode("Exception")) - else: + when chronosStrictException: raises.add(newIdentNode("CatchableError")) when (NimMajor, NimMinor) < (1, 4): raises.add(newIdentNode("Defect")) + else: + raises.add(newIdentNode("Exception")) closureIterator.addPragma(nnkExprColonExpr.newTree( newIdentNode("raises"), @@ -328,5 +328,5 @@ macro async*(prc: untyped): untyped = result.add asyncSingleProc(oneProc) else: result = asyncSingleProc(prc) - when defined(nimDumpAsync): + when chronosDumpAsync: echo repr result diff --git a/chronos/config.nim b/chronos/config.nim new file mode 100644 index 0000000..abc9c37 --- /dev/null +++ b/chronos/config.nim @@ -0,0 +1,57 @@ +## Compile-time configuration options for chronos that control the availability +## of various strictness and debuggability options. In general, debug helpers +## are enabled when `debug` is defined while strictness options are introduced +## in transition periods leading up to a breaking release that starts enforcing +## them and removes the option. +## +## `chronosPreviewV4` is a preview flag to enable v4 semantics - in particular, +## it enables strict exception checking and disables parts of the deprecated +## API and other changes being prepared for the upcoming release +## +## `chronosDebug` can be defined to enable several debugging helpers that come +## with a runtime cost - it is recommeneded to not enable these in production +## code. +when (NimMajor, NimMinor) >= (1, 4): + const + chronosStrictException* {.booldefine.}: bool = defined(chronosPreviewV4) + ## Require that `async` code raises only derivatives of `CatchableError` and + ## not `Exception` - forward declarations, methods and `proc` types used + ## from within `async` code may need to be be explicitly annotated with + ## `raises: [CatchableError]` when this mode is enabled. + + chronosStackTrace* {.booldefine.}: bool = defined(chronosDebug) + ## Include stack traces in futures for creation and completion points + + chronosFutureId* {.booldefine.}: bool = defined(chronosDebug) + ## Generate a unique `id` for every future - when disabled, the address of + ## the future will be used instead + + chronosFutureTracking* {.booldefine.}: bool = defined(chronosDebug) + ## Keep track of all pending futures and allow iterating over them - + ## useful for detecting hung tasks + + chronosDumpAsync* {.booldefine.}: bool = defined(nimDumpAsync) + ## Print code generated by {.async.} transformation +else: + # 1.2 doesn't support `booldefine` in `when` properly + const + chronosStrictException*: bool = + defined(chronosPreviewV4) or defined(chronosStrictException) + chronosStackTrace*: bool = defined(chronosDebug) or defined(chronosStackTrace) + chronosFutureId*: bool = defined(chronosDebug) or defined(chronosFutureId) + chronosFutureTracking*: bool = + defined(chronosDebug) or defined(chronosFutureTracking) + chronosDumpAsync*: bool = defined(nimDumpAsync) + +when defined(debug) or defined(chronosConfig): + import std/macros + + static: + hint("Chronos configuration:") + template printOption(name: string, value: untyped) = + hint(name & ": " & $value) + printOption("chronosStrictException", chronosStrictException) + printOption("chronosStackTrace", chronosStackTrace) + printOption("chronosFutureId", chronosFutureId) + printOption("chronosFutureTracking", chronosFutureTracking) + printOption("chronosDumpAsync", chronosDumpAsync) diff --git a/chronos/debugutils.nim b/chronos/debugutils.nim index 451189a..17d6412 100644 --- a/chronos/debugutils.nim +++ b/chronos/debugutils.nim @@ -12,10 +12,10 @@ when (NimMajor, NimMinor) < (1, 4): else: {.push raises: [].} -import ./asyncloop +import "."/[asyncloop, config] export asyncloop -when defined(chronosFutureTracking): +when chronosFutureTracking: import stew/base10 const @@ -34,7 +34,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. - when defined(chronosFutureTracking): + when chronosFutureTracking: var count = 0'u var res = "" for item in pendingFutures(): @@ -62,7 +62,7 @@ proc pendingFuturesCount*(filter: set[FutureState]): uint = ## ## If ``filter`` is equal to ``AllFutureStates`` Operation's complexity is ## O(1), otherwise operation's complexity is O(n). - when defined(chronosFutureTracking): + when chronosFutureTracking: if filter == AllFutureStates: pendingFuturesCount() else: diff --git a/tests/testaddress.nim b/tests/testaddress.nim index 040fc28..e505ff5 100644 --- a/tests/testaddress.nim +++ b/tests/testaddress.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} suite "TransportAddress test suite": test "initTAddress(string)": diff --git a/tests/testasyncstream.nim b/tests/testasyncstream.nim index b8c729b..fd581cb 100644 --- a/tests/testasyncstream.nim +++ b/tests/testasyncstream.nim @@ -10,7 +10,7 @@ import bearssl/[x509] import ../chronos import ../chronos/streams/[tlsstream, chunkstream, boundstream] -when defined(nimHasUsed): {.used.} +{.used.} # To create self-signed certificate and key you can use openssl # openssl req -new -x509 -sha256 -newkey rsa:2048 -nodes \ diff --git a/tests/testbugs.nim b/tests/testbugs.nim index d31ea99..19a8edb 100644 --- a/tests/testbugs.nim +++ b/tests/testbugs.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} suite "Asynchronous issues test suite": const HELLO_PORT = 45679 diff --git a/tests/testdatagram.nim b/tests/testdatagram.nim index 149ce9d..1eea489 100644 --- a/tests/testdatagram.nim +++ b/tests/testdatagram.nim @@ -9,7 +9,7 @@ import std/[strutils, net] import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} suite "Datagram Transport test suite": const diff --git a/tests/testfut.nim b/tests/testfut.nim index 23015b9..fa250a1 100644 --- a/tests/testfut.nim +++ b/tests/testfut.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos, ../chronos/unittest2/asynctests -when defined(nimHasUsed): {.used.} +{.used.} suite "Future[T] behavior test suite": proc testFuture1(): Future[int] {.async.} = diff --git a/tests/testhttpclient.nim b/tests/testhttpclient.nim index 15f77d3..e04e2ab 100644 --- a/tests/testhttpclient.nim +++ b/tests/testhttpclient.nim @@ -10,7 +10,7 @@ import unittest2 import ../chronos, ../chronos/apps/http/[httpserver, shttpserver, httpclient] import stew/base10 -when defined(nimHasUsed): {.used.} +{.used.} # To create self-signed certificate and key you can use openssl # openssl req -new -x509 -sha256 -newkey rsa:2048 -nodes \ diff --git a/tests/testhttpserver.nim b/tests/testhttpserver.nim index b200040..8b71ca2 100644 --- a/tests/testhttpserver.nim +++ b/tests/testhttpserver.nim @@ -11,7 +11,7 @@ import ../chronos, ../chronos/apps/http/httpserver, ../chronos/apps/http/httpcommon import stew/base10 -when defined(nimHasUsed): {.used.} +{.used.} suite "HTTP server testing suite": type diff --git a/tests/testmacro.nim b/tests/testmacro.nim index e6c9812..f50015f 100644 --- a/tests/testmacro.nim +++ b/tests/testmacro.nim @@ -9,7 +9,7 @@ import unittest2 import macros import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} type RetValueType = proc(n: int): Future[int] {.async.} diff --git a/tests/testnet.nim b/tests/testnet.nim index ff78429..419195d 100644 --- a/tests/testnet.nim +++ b/tests/testnet.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos/transports/[osnet, ipnet] -when defined(nimHasUsed): {.used.} +{.used.} suite "Network utilities test suite": diff --git a/tests/testratelimit.nim b/tests/testratelimit.nim index 32b8e95..4c78664 100644 --- a/tests/testratelimit.nim +++ b/tests/testratelimit.nim @@ -6,7 +6,9 @@ # Apache License, version 2.0, (LICENSE-APACHEv2) # MIT license (LICENSE-MIT) -import unittest +{.used.} + +import unittest2 import ../chronos import ../chronos/ratelimit diff --git a/tests/testserver.nim b/tests/testserver.nim index 8828bb2..e7e834e 100644 --- a/tests/testserver.nim +++ b/tests/testserver.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} suite "Server's test suite": type diff --git a/tests/testshttpserver.nim b/tests/testshttpserver.nim index 596a8da..4b55b27 100644 --- a/tests/testshttpserver.nim +++ b/tests/testshttpserver.nim @@ -10,7 +10,7 @@ import unittest2 import ../chronos, ../chronos/apps/http/shttpserver import stew/base10 -when defined(nimHasUsed): {.used.} +{.used.} # To create self-signed certificate and key you can use openssl # openssl req -new -x509 -sha256 -newkey rsa:2048 -nodes \ diff --git a/tests/testsignal.nim b/tests/testsignal.nim index 1163a8c..5eca5a9 100644 --- a/tests/testsignal.nim +++ b/tests/testsignal.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} when not defined(windows): import posix diff --git a/tests/testsoon.nim b/tests/testsoon.nim index 8b76113..69bffd4 100644 --- a/tests/testsoon.nim +++ b/tests/testsoon.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} suite "callSoon() tests suite": const CallSoonTests = 10 diff --git a/tests/teststream.nim b/tests/teststream.nim index e0ee9f3..90fd55d 100644 --- a/tests/teststream.nim +++ b/tests/teststream.nim @@ -9,7 +9,7 @@ import std/[strutils, os] import unittest2 import ".."/chronos, ".."/chronos/osdefs -when defined(nimHasUsed): {.used.} +{.used.} when defined(windows): proc get_osfhandle*(fd: FileHandle): HANDLE {. diff --git a/tests/testsync.nim b/tests/testsync.nim index 9acea50..4e09682 100644 --- a/tests/testsync.nim +++ b/tests/testsync.nim @@ -8,7 +8,7 @@ import unittest2 import ../chronos -when defined(nimHasUsed): {.used.} +{.used.} suite "Asynchronous sync primitives test suite": var testLockResult {.threadvar.}: string diff --git a/tests/testtime.nim b/tests/testtime.nim index 430db8f..aac926e 100644 --- a/tests/testtime.nim +++ b/tests/testtime.nim @@ -9,7 +9,7 @@ import std/os import unittest2 import ../chronos, ../chronos/timer -when defined(nimHasUsed): {.used.} +{.used.} static: doAssert Moment.high - Moment.low == Duration.high diff --git a/tests/testutils.nim b/tests/testutils.nim index 67a59ab..f458190 100644 --- a/tests/testutils.nim +++ b/tests/testutils.nim @@ -6,12 +6,12 @@ # Apache License, version 2.0, (LICENSE-APACHEv2) # MIT license (LICENSE-MIT) import unittest2 -import ../chronos +import ../chronos, ../chronos/config -when defined(nimHasUsed): {.used.} +{.used.} suite "Asynchronous utilities test suite": - when defined(chronosFutureTracking): + when chronosFutureTracking: proc getCount(): uint = # This procedure counts number of Future[T] in double-linked list via list # iteration. @@ -21,7 +21,7 @@ suite "Asynchronous utilities test suite": res test "Future clean and leaks test": - when defined(chronosFutureTracking): + when chronosFutureTracking: if pendingFuturesCount(WithoutFinished) == 0'u: if pendingFuturesCount(OnlyFinished) > 0'u: poll() @@ -33,7 +33,7 @@ suite "Asynchronous utilities test suite": skip() test "FutureList basics test": - when defined(chronosFutureTracking): + when chronosFutureTracking: var fut1 = newFuture[void]() check: getCount() == 1'u @@ -65,7 +65,7 @@ suite "Asynchronous utilities test suite": skip() test "FutureList async procedure test": - when defined(chronosFutureTracking): + when chronosFutureTracking: proc simpleProc() {.async.} = await sleepAsync(10.milliseconds)