simplify lock/semaphore, deprecate AsyncLockError/AsyncSemaphoreError

[arnetheduck]

AsyncLock is a special case of AsyncSemaphore (with size 1) - make sure that they both behave the same.

In particular, simpify the AsyncLock implementation to use fewer poll rounds and futures during acquire while maintaining the same fairness guarantees as before.

At the same time, deprecate AsyncLockError and AsyncSemaphoreError and introduce release2 that panics instead of raising a checked exception when trying to release a lock / semaphore that is not held - this behavior matches Lock in the stdlib and avoids an annoying try/except in finally for code that uses raises: [].

release2 is added as an interim solution for code that wants to avoid both warning and exception handler while waiting for a new chronos major release that changes the existing release function to the new behavior.

[arnetheduck]

This PR is an alternative to #718

[etan-status]

Preferring this one over #718, as it's a logic bug to release something that wasn't acquired. The solution is to fix the logic bug, not to add workarounds just in case that there is a specific class of logic bugs (that would just hide them).

I don't think it needs a new name though. any callers that use a surrounding try / except will simply get a warning that the release can't raise anymore.

[arnetheduck]

will simply get a warning that the release can't raise anymore.

not quite - if they were relying on the exception to be raised, they'll get a runtime panic instead - not great.

[etan-status]

is there a correct way to handle it other than raiseAssert "should not be reached" inside the except?

https://github.com/vacp2p/nim-quic/blob/main/quic/transport/stream.nim#L94
this one just silences the buggy logic, not good either

https://github.com/logos-messaging/logos-delivery/blob/master/logos_delivery/channels/scalable_data_sync/scalable_data_sync.nim#L241
this one forwards it to the caller, making the problem worse as now a potential logic bug gets conflated with actual runtime bugs (the lock doesn't magically get fixed, if usage was wrong)

other usage I found in a brief search was raiseAssert, or within tests

[arnetheduck]

is there a correct way

not that I can think of 🤷 but consider a "task runner" - it might have a policy to keep the system running even though any one task fails - it's dubious if such systems should be built without process isolation but that's the one case I can think of where you would gracefully swallow a "logic error" and keep going. the asynclock feels like an unlikely mechnism for such a system, regardless

[nitely]

I think it is probably better to deprecate this function. What if the user does not use AsyncLockError to catch errors? ie: no AsyncLockError deprecation warning.

[arnetheduck]

the aim is to get to a world where release remains the canonical name for this function is the same as for the regular.. it's a good point about never catching the error, but the runtime defect will have to do if someone is using release in this strange way.

[etan-status]

Such a task runner would also work better by just crashing and restarting (even by systemd) to restore a consistent state. I don't see why releasing a semaphore needs to optimize for API misuse, while accidentally returning a nil future in an {.async: (raw: true).} early return, accidentally doing a newSeqUninit(2^47 for segfault, 2^40 for defect), or dereferencing a nil ptr, are all sources that can also randomly crash the program if using the APIs incorrectly.

One can probably more generically catch the sigsegv, random sigbus stuff, floating point div by 0, or the defect, to allow saving unsaved documents. Or use aforementioned process isolation. But catching some logic bugs, while being unable to deal with other logic bugs, not sure if that's worth extra API surface.