This project is read-only.
1
Vote

Wrong TimeSpan usage

description

In a lot of places, you di timeSpan.Milliseconds where timeSpan.TotalMilliseconds should be used. For e.g. TimeSpan.FormMinutes(1), Milliseconds is 0, but TotalMilliseconds is 60000.
Please see the attached patch.

comments

mormayo wrote Sep 22, 2011 at 4:58 PM

I have deleted the attachment as it's not a recognized file format. If you have code, then attach a code file or copy-paste code to this issue. There is not much context on this item to do anything with it. What projects or files are you referring to?

etobi wrote Sep 22, 2011 at 5:39 PM

Are you kidding me? *.patch is no recognized file format?

And if I'm posting an issue on chesstool.codeplex.com it's obvious that I'm, referring to the Chess project, isn't it? And all the buggy files were listed in the *.patch file. I'm not going to upload this again, so please take a look here:

http://pastie.org/2574671

mormayo wrote Sep 22, 2011 at 6:11 PM

Ah, some searching on Google tells me that patch looks like an SVN thing. Please forgive me as I do not have SVN installed (I use TFS) and hadn't heard of it before. I've pasted the contents below: Index: Framework4Wrappers/Task.cs =================================================================== --- Framework4Wrappers/Task.cs (revision 62424) +++ Framework4Wrappers/Task.cs (working copy) @@ -219,7 +219,7 @@ } public static bool Wait(Original::Task self, TimeSpan timeout) { - return WaitRaw(self, timeout.Milliseconds, CancellationToken.None); + return WaitRaw(self, (int) timeout.TotalMilliseconds, CancellationToken.None); } public static bool Wait(Original::Task self, int millisecondsTimeout, CancellationToken cancellationToken) { @@ -349,7 +349,7 @@ } public static bool WaitAll(Original::Task[] tasks, TimeSpan timeout) { - return WaitAllRaw(tasks, timeout.Milliseconds, CancellationToken.None); + return WaitAllRaw(tasks, (int) timeout.TotalMilliseconds, CancellationToken.None); } public static bool WaitAll(Original::Task[] tasks, int millisecondsTimeout, CancellationToken cancellationToken) { @@ -369,7 +369,7 @@ } public static int WaitAny(Original::Task[] tasks, TimeSpan timeout) { - return WaitAnyRaw(tasks, timeout.Milliseconds, CancellationToken.None); + return WaitAnyRaw(tasks, (int) timeout.TotalMilliseconds, CancellationToken.None); } public static int WaitAny(Original::Task[] tasks, int millisecondsTimeout, CancellationToken cancellationToken) { Index: Microsoft.ManagedChess.Framework35Wrappers/readerwriterlockslim.cs =================================================================== --- Microsoft.ManagedChess.Framework35Wrappers/readerwriterlockslim.cs (revision 62424) +++ Microsoft.ManagedChess.Framework35Wrappers/readerwriterlockslim.cs (working copy) @@ -72,7 +72,7 @@ public static bool TryEnterReadLock(Original::ReaderWriterLockSlim @lock, TimeSpan timeSpan) { - return TryEnterRaw(@lock, timeSpan.Milliseconds, "TryEnterReadLock", MODE.READ); + return TryEnterRaw(@lock, (int) timeSpan.TotalMilliseconds, "TryEnterReadLock", MODE.READ); } public static bool TryEnterWriteLock(Original::ReaderWriterLockSlim @lock, int millisecondsTimeout) @@ -82,7 +82,7 @@ public static bool TryEnterWriteLock(Original::ReaderWriterLockSlim @lock, TimeSpan timeSpan) { - return TryEnterRaw(@lock, timeSpan.Milliseconds, "TryEnterWriteLock", MODE.WRITE); + return TryEnterRaw(@lock, (int) timeSpan.TotalMilliseconds, "TryEnterWriteLock", MODE.WRITE); } public static bool TryEnterUpgradeableReadLock(Original::ReaderWriterLockSlim @lock, int millisecondsTimeout) @@ -92,7 +92,7 @@ public static bool TryEnterUpgradeableReadLock(Original::ReaderWriterLockSlim @lock, TimeSpan ts) { - return TryEnterRaw(@lock, ts.Milliseconds, "TryEnterUpgradeableReadLock", MODE.UPGRADEABLE_READ); + return TryEnterRaw(@lock, (int) ts.TotalMilliseconds, "TryEnterUpgradeableReadLock", MODE.UPGRADEABLE_READ); } public static bool TryEnterRaw(Original::ReaderWriterLockSlim @lock, int millisecondsTimeout, string name, MODE mode) Index: ThreadingWrappers/Monitor.cs =================================================================== --- ThreadingWrappers/Monitor.cs (revision 62424) +++ ThreadingWrappers/Monitor.cs (working copy) @@ -91,15 +91,15 @@ // an exclusive lock on the specified object. public static bool TryEnter(object @lock, TimeSpan timeOut) { - MSyncVarOp mop = (timeOut.Milliseconds == Timeout.Infinite) ? MSyncVarOp.LOCK_ACQUIRE : MSyncVarOp.LOCK_TRYACQUIRE; - return TryEnterRaw(@lock, timeOut.Milliseconds, mop, "Monitor.TryEnter"); + MSyncVarOp mop = (timeOut.TotalMilliseconds == Timeout.Infinite) ? MSyncVarOp.LOCK_ACQUIRE : MSyncVarOp.LOCK_TRYACQUIRE; + return TryEnterRaw(@lock, (int) timeOut.TotalMilliseconds, mop, "Monitor.TryEnter"); } public static void TryEnter(object obj, TimeSpan millisecondsTimeout, ref bool tookLock) { global::System.Diagnostics.Debug.Assert(!tookLock); - MSyncVarOp mop = (millisecondsTimeout.Milliseconds == Timeout.Infinite) ? MSyncVarOp.LOCK_ACQUIRE : MSyncVarOp.LOCK_TRYACQUIRE; - tookLock = TryEnterRaw(obj, millisecondsTimeout.Milliseconds, mop, "Monitor.TryEnter"); + MSyncVarOp mop = (millisecondsTimeout.TotalMilliseconds == Timeout.Infinite) ? MSyncVarOp.LOCK_ACQUIRE : MSyncVarOp.LOCK_TRYACQUIRE; + tookLock = TryEnterRaw(obj, (int) millisecondsTimeout.TotalMilliseconds, mop, "Monitor.TryEnter"); } // Monitor.TryEnter (Object) Attempts to acquire an exclusive lock on the specified object. @@ -281,7 +281,7 @@ // - If the specified time-out interval elapses, the thread enters the ready queue. public static bool Wait(object @lock, TimeSpan timeOut) { - return WaitRaw(@lock, timeOut.Milliseconds, false); + return WaitRaw(@lock, (int) timeOut.TotalMilliseconds, false); } // Monitor.Wait (Object, Int32, Boolean) @@ -301,7 +301,7 @@ // - reacquires the domain afterward. public static bool Wait(object @lock, TimeSpan timeOut, bool domain) { - return WaitRaw(@lock, timeOut.Milliseconds, domain); + return WaitRaw(@lock, (int) timeOut.TotalMilliseconds, domain); } private static bool WaitRaw(object @lock, int timeOut, bool domain) Index: ThreadingWrappers/Mutex.cs =================================================================== --- ThreadingWrappers/Mutex.cs (revision 62424) +++ ThreadingWrappers/Mutex.cs (working copy) @@ -71,7 +71,7 @@ public static bool WaitOne(Original::Mutex mutex, TimeSpan timeOut, bool exitContext) { - return WaitOneRaw(mutex, timeOut.Milliseconds, exitContext); + return WaitOneRaw(mutex, (int) timeOut.TotalMilliseconds, exitContext); } private static bool WaitOneRaw(Original::Mutex mutex, int timeOut, bool domain) Index: ThreadingWrappers/Thread.cs =================================================================== --- ThreadingWrappers/Thread.cs (revision 62424) +++ ThreadingWrappers/Thread.cs (working copy) @@ -66,7 +66,7 @@ Helper.SimpleWrap<bool>( delegate(ClrSyncManager manager) { - if (timeout.Milliseconds < Timeout.Infinite) + if (timeout.TotalMilliseconds < Timeout.Infinite) throw new ArgumentOutOfRangeException(); // sleep with TimeSpan cannot be infinite manager.TaskYield(); @@ -137,7 +137,7 @@ public static bool Join(Original::Thread thread, TimeSpan timespan) { return Helper.SimpleWrap<bool>( - delegate(ClrSyncManager m) { return JoinRaw(thread, m, timespan.Milliseconds); }, + delegate(ClrSyncManager m) { return JoinRaw(thread, m, (int) timespan.TotalMilliseconds); }, delegate() { return thread.Join(timespan);} ); } Index: ThreadingWrappers/threadpool.cs =================================================================== --- ThreadingWrappers/threadpool.cs (revision 62424) +++ ThreadingWrappers/threadpool.cs (working copy) @@ -219,7 +219,7 @@ TimeSpan timeOut, bool executeOnlyOnce) { - return RegisterWaitForSingleObjectHelper(waitObject, callBack, state, timeOut.Milliseconds, executeOnlyOnce, true); + return RegisterWaitForSingleObjectHelper(waitObject, callBack, state, (long) timeOut.TotalMilliseconds, executeOnlyOnce, true); } public static Original.RegisteredWaitHandle RegisterWaitForSingleObject( @@ -259,7 +259,7 @@ TimeSpan ts, bool executeOnlyOnce) { - return RegisterWaitForSingleObjectHelper(waitObject, callBack, state, ts.Milliseconds, executeOnlyOnce, false); + return RegisterWaitForSingleObjectHelper(waitObject, callBack, state, (long) ts.TotalMilliseconds, executeOnlyOnce, false); } public static Original.RegisteredWaitHandle UnsafeRegisterWaitForSingleObject( Index: ThreadingWrappers/Timer.cs =================================================================== --- ThreadingWrappers/Timer.cs (revision 62424) +++ ThreadingWrappers/Timer.cs (working copy) @@ -232,9 +232,9 @@ public static Original::Timer ___ctor_newobj(Original::TimerCallback tc, object state, TimeSpan dueTime, TimeSpan period) { - if (dueTime.Milliseconds< 0 && dueTime.Milliseconds != Timeout.Infinite) + if (dueTime.TotalMilliseconds< 0 && dueTime.TotalMilliseconds != Timeout.Infinite) throw new ArgumentOutOfRangeException(); - if (period.Milliseconds < 0 && period.Milliseconds != Timeout.Infinite) + if (period.TotalMilliseconds < 0 && period.TotalMilliseconds != Timeout.Infinite) throw new ArgumentOutOfRangeException(); return Helper.SimpleWrap<Original::Timer>( delegate(ClrSyncManager manager) { return TimerHelpers.CreateTimer(manager, tc, state, dueTime, period); }, @@ -328,7 +328,7 @@ public static bool Change(Original::Timer t, TimeSpan dueTime, TimeSpan period) { - return Change(t, (long)dueTime.Milliseconds, (long)period.Milliseconds); + return Change(t, (long)dueTime.TotalMilliseconds, (long)period.TotalMilliseconds); } public static bool Change(Original::Timer t, uint dueTime, uint period) Index: ThreadingWrappers/WaitHandle.cs =================================================================== --- ThreadingWrappers/WaitHandle.cs (revision 62424) +++ ThreadingWrappers/WaitHandle.cs (working copy) @@ -137,7 +137,7 @@ { if (h == null) throw new NullReferenceException(); - return WaitOne(h, timeout.Milliseconds, exitContext); + return WaitOne(h, (int) timeout.TotalMilliseconds, exitContext); } public static int WaitAny(Original::WaitHandle[] waitHandles) @@ -162,7 +162,7 @@ public static int WaitAny(Original::WaitHandle[] waitHandles, TimeSpan timeout, bool exitContext) { - return WaitAny(waitHandles, timeout.Milliseconds, exitContext); + return WaitAny(waitHandles, (int) timeout.TotalMilliseconds, exitContext); } public static bool WaitAll(Original::WaitHandle[] waitHandles) @@ -187,7 +187,7 @@ public static bool WaitAll(Original::WaitHandle[] waitHandles, TimeSpan timeout, bool exitContext) { - return WaitAll(waitHandles, timeout.Milliseconds, exitContext); + return WaitAll(waitHandles, (int) timeout.TotalMilliseconds, exitContext); } public static bool SignalAndWait(Original::WaitHandle toSignal, Original::WaitHandle toWaitOn) @@ -243,7 +243,7 @@ TimeSpan timeout, bool exitContext) { - return SignalAndWait(toSignal, toWaitOn, timeout.Milliseconds, exitContext); + return SignalAndWait(toSignal, toWaitOn, (int) timeout.TotalMilliseconds, exitContext); } }

etobi wrote Sep 22, 2011 at 6:55 PM

It's not just a SVN thing .patch/.diff files are used for a looooong time to represent changes in plain text files. I think TFS can create patches with tf diff as well - but, I haven't touched TFS for a long time in favour to Git.

Anyways: The changes at least fix an issue with some false-positive deadlock-warnings when e.g. Monitor.Wait(@lock, TimeSpan.FromHours(1)) is used (where .Milliseconds would return 0).
But after fixing this I ran into another issue - seems Chess doesn't track all threads resulting in one thread using the original Monitor.Wait() method and other threads to use the modified version of Monitor.PulseAll(), causing the waiting thread to never wake up.

wrote Feb 14, 2013 at 8:24 PM