Interval

[dryajov]

Adds an interval primitives triggered at specific intervals of Duration.

Additionally, adds a missing addTimer with Duration instead of Moment, which is deprecated.

[zah]

What are the desired exception-safety semantics here? This should be documented.
My suggestion would be to specify raises: [Defect] on the CallbackFunc which would force the user to handle all recoverable errors in their callback body.

[dryajov]

@zah 👍 . This would require a bit of refactoring and it should be done in it's own issue, since it would affect all chronos callbacks, furthermore this would imply fixing any project upstream that isn't doing proper exception handling inside the callback. I'd like to hear @cheatfate opinion on this.

[arnetheduck]

raises: [Defect]

for the record, this remains logically unsound - when a defect happens, by definition the world is in an inconsistent state and cannot be consistently unwound. this is an intrinsic problem with the definition of Defect and the desire to reason about the state of the application when it happens. adding code like this creates a deeper problem down the road creating a false sense of security that the code dealing with the callback somehow supports defects - it doesn't.

you cannot create a sound reasoning around Defect as it's currently defined and works in the compiler - it's inherently defective (hahahaha) - to work around it, perhaps we should create a {.noraises.} alias that does {.raises: Defect.} for now until the compiler has a sound implementation of Defect because allowing defects through sadly is the least bad option - at least it's documented.

this would imply fixing any project upstream that isn't doing proper exception handling inside the callback

a callback that allows an exception to escape through the callback is already broken (it breaks the state of the read loop), thus nothing is broken "more" by this.

[cheatfate]

CallbackFunc as well as addTimer/setTimer/addReader/addWriter are actually internal procedures and should not be public, but because Nim do not supports "partially" public procedures i can't make it private and use it in other modules. But this API is not supposed to be used by consumer applications.

sleepAsync(), wait(), withTimeout() should be enough to implement all the consumer logic and we can add more utility functions here.

[zah]

@arnetheduck, this was already discussed at length. There are merits to tracking defects and Araq still hasn't decided what would be the way forward. For the time being, Defects are tracked and we must work under the current semantics.

I personally feel your thinking is influenced too much by analogies with "undefined behavior" in C. In a memory-safe language, a module can use defensive coding with sentry objects to provide safe unwinding in the face of unanticipated defects. Under specific circumstances, it may be reasonable to handle a defect from a module if the likelihood of poisoning your application state is low (e.g. in a GUI e-mail client, it may be reasonable to handle a defect from the zip module when you try to decompress an attachment).

Regarding the noraises pragma, I've already proposed a pragma called errors as a more general mechanism for hiding the details regarding the tracking of Defects.

[zah]

In our particular case, the propagation of Defect though this callback is appropriate, because it will ultimately kill the event loop and terminate the application - something we want to happen when a Defect is being encountered.

[arnetheduck]

Under specific circumstances

... where the Defect by definition is no longer a Defect, because you've reasoned about the state and decided it's fine after all.

In a memory-safe language

which nim is not (no more than c++) - also, memory is one kind of resource, but there are many others: locks, handles, files, etc. it is reasoning about these where the difficulty begins, not trivial, well-understood cases (in which case Defect is unlikely to be the right thing to raise. we can get away with this sloppy mentality for now for example, because we're not doing threads, so many of the issues are swept under the rug. it doesn't make reasoning about Defect any more sound.

I've already proposed

so let's get it done - at least as documentation? fits stew perfectly, doesn't need a sound implementation, can be a simple alias to raises for all I care. at least we're not creating false expectations of safe and sound code that way.

[zah]

the Defect by definition is no longer a Defect

I wish you'd start using the definition of Defect that I offered several times already. A Defect is a violation of the contract of a particular API (e.g. a violated pre-condition). This is a very precise definition, no ambiguity there.

What you reason about is how isolated certain components are, how mission critical their functionality is. Crashing on the user machine has a downside too, so you may end up accepting that Defects in certain components can be handled with graceful degradation. Obviously, this is risky business and if you can get away with process isolation, that's better, but also not always easy to achieve.

which nim is not (no more than c++)

Nim still has significant amount of undefined behaviour (which I hope will be reduced in the future), but when you hit UB, you are not raising Defects. Defects are raised when the language safely prevented you from going into UB. They trigger the unwinding logic prepared by the programmer. We can examine this logic and conclude that a piece of code is providing strong safety, basic safety or no safety at all in the face of exceptions. And let's not kid ourselves, using destructors, defer or finally is the only way to provide such guarantees and the code has to be written correctly regardless of whether we are discussing the recoverable exit paths or the defect-driven ones. In other words, it's quite possible to write correct unwinding logic even when you cannot anticipate where a defect might happen.

[cheatfate]

Why we need such primitive? And why not use:

proc runWithTimeInterval(time: Duration, callback: AnyOtherCallbackType): Future[void] =
  while true:
    await sleepAsync(time)
    if not callback():
      break

proc someProc() {.async.} =
  asyncCheck runWithTimeInterval(1.seconds, someProcedure)

[cheatfate]

One more thing, your code is using already deprecated primitives.

[dryajov]

Why we need such primitive? And why not use:

I need to schedule callbacks at intervals.

One more thing, your code is using already deprecated primitives.

This been sitting in review for some time, you just got around to reviewing it. Don't remember, but my best guess is that they weren't deprecated at the time or some other limitation of the implementation.

Also, I'd appreciate a more promptly response for the PRs next time. It's been almost two months and I just got your attention, meanwhile this had to be hacked into libp2p directly.

[dryajov]

Also, if addTimer is internal, how should callbacks triggered in the future be scheduled? Neither of sleepAsync(), wait(), withTimeout(), allow you to schedule things to run in the future AFAIS?

[cheatfate]

@dryajov you can check my code example how to schedule code to be running in future.

  await sleepAsync(someTime)
  # schedule your code here

or if you need to schedule your code periodically you can do

  while true:
    await sleepAsync(someTime)
    # schedule your code here

Sorry, that i have not answered you in time.

[dryajov]

@cheatfate I don't think your examples will work. This assume you need to wait for something to execute, I don't need to wait for it, I need this to be triggered at some point in the future, like a cleanup task or something similar. That's why addTimer/setTimer and addInterval/setInterval are core scheduling primitives, you've something like this available in every language that implements the asynchronous paradigm.

[dryajov]

I mean, runWithTimeInterval would definitely work, but this should be part of chronos, addTimer should be kept public as well, as there is no alternative to it.

[cheatfate]

python.asyncio do not have interval functions as part of core library.
boost.asio do not have interval functions as part of core library.
Why chronos should have such primitives?

[bung87]

any updates? I need api match std asyncdispatch's addTimer(oneshot=false)