nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.47k stars 1.47k forks source link

Deadloop in Windows, `os.sleep(-1)` #23732

Closed litlighilit closed 3 months ago

litlighilit commented 3 months ago

Description

when defined(windows):
  import std/os
  sleep(-1)  # <- deadloop

BTW, on POSIX, it just returns immediately, as sleep calls nanosleep, which exits with errno set as EINVAL for negative values.

Nim Version

At least from 1.6.14 to 2.1.1 and devel

Current Output

No response

Expected Output

No response

Possible Solution

Two direction:

  1. raises a exception on all platforms when milsecs is negative.
  2. add check in Windows, returns immediately for negative values

Additional Information

In Python, time.sleep raises ValueError on negative value.

Maybe we can raises OSError as Nim's sleep is in std/os if the first direction is accepted.

On the other hand, golang's time.Sleep will return immediately for negative values.

litlighilit commented 3 months ago

BTW, It seems to deadloop as os.sleep calls Sleep on Windows, whose parameter type, is in fact DWORD, an uint32, but Nim wraps it as int32, so pass negative int just becomes a large unsigned int.

Still a DWORD relative issue...

Araq commented 3 months ago

What's wrong with -1 meaning "forever"?

litlighilit commented 3 months ago

on posix, it returns immediately. (and set errno to EINVAL)

litlighilit commented 3 months ago

What's wrong with -1 meaning "forever"?

And strictly speaking, not forever, but a very large uint amount of time.

e.g. Currently on Windows: sleep(-1) in fact does Sleep(4294967295)

a.k.a. while sleep(-1) means sleep forever is somewhat reasonable, it means sleep for 49 days on Windows and sleep for no time on POSIX.

juancarlospaco commented 3 months ago

sleep should take Natural.