Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.Threading.Thread.Sleep method does not sleep for milliseconds specified in certain cases #49

Open
laimis opened this issue Apr 23, 2023 · 1 comment
Labels
design is:bug Something isn't working pri:high

Comments

@laimis
Copy link

laimis commented Apr 23, 2023

ThreadJob class uses System.Threading.Thread.Sleep, which has issues sleeping the right number of milliseconds provided in certain scenarios. For more information please see:

https://stackoverflow.com/questions/19066900/thread-sleep1-takes-longer-than-1ms

We discovered this issue in Lucene.NET repo with some slow running tests and the discussion for that can be seen here:

apache/lucenenet#838

@NightOwl888 NightOwl888 added is:bug Something isn't working design pri:high labels Apr 23, 2023
@NightOwl888
Copy link
Owner

This implementation seems to work in the case of apache/lucenenet#838.

internal static void Sleep(int milliseconds)
{
    long ticks = (long)((Stopwatch.Frequency / (double)1000) * milliseconds); // ticks per millisecond * milliseconds = total delay ticks
    long initialTick = Stopwatch.GetTimestamp();
    long targetTick = initialTick + ticks;
    while (Stopwatch.GetTimestamp() < targetTick)
    {
        Thread.SpinWait(1);
    }
}

However, we need to confirm that Thread.SpinWait() will throw a ThreadInterruptedException when Thread.Interrupt() is called by another thread. Unfortunately, the docs for the Thread.Sleep() method don't mention the exception even though it is definitely thrown here, so it is also unclear whether Thread.SpinWait() will throw this.

The TestThreadInterruptDeadlock() test sets up a scenario to force this to happen (which is not very easy to do). We can use it to confirm whether the ThreadInterruptedException is thrown and if not, verify a potential solution for doing so.

After the ThreadJob.Sleep() overloads are fixed with a more precise implementation, Lucene.NET should use it instead of Thread.Sleep() in most (if not all) cases. This is used mostly in tests, but there are also a few IndexWriter and other production code calls where this might be of benefit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design is:bug Something isn't working pri:high
Projects
None yet
Development

No branches or pull requests

2 participants