Test more invalid values for ThreadPool.[Unsafe]RegisterWaitForSingleObject

[filipnavara]

Unit tests for dotnet/corert#6887.

[stephentoub]

None of the existing ones or the new ones are validating the argument name stored in the exception. There is one in all of these cases, right? Can these checks be augmented to include that? That would likely mean changing them all to be AssertExtensions.Throws instead of Assert.Throws.

[filipnavara]

Good point. I will add it.

[filipnavara]

For some of the ArgumentNullExceptions CoreRT and CoreCLR return different values. CoreRT correctly returns the actual parameter names (eg. "callBack"). CoreCLR mimics the legacy value from .NET Framework even though it's technically wrong ("WaitOrTimerCallback").

I'll add it to places where it's not a problem, but I would prefer the rest to be addressed separately because it will either need exceptions for certain frameworks or modifying the behavior of CoreRT or CoreCLR.

[danmoseley]

We should ideally fix CoreCLR's name - it's OK to correct it.

[filipnavara]

I'll submit the CoreCLR fix and amend this PR.

[stephentoub]

One comment, otherwise LGTM.

[stephentoub]

@dotnet-bot test this please

[filipnavara]

It was broken on NetFX last time I checked. It may need a backport of the CLR fix.

[stephentoub]

It may need a backport of the CLR fix.

That's unlikely to happen. You'll need to adjust the test.

[filipnavara]

That's unlikely to happen. You'll need to adjust the test.

Only thing I can do is to split it in two and skip the problematic part on NetFX.

[stephentoub]

Only thing I can do is to split it in two and skip the problematic part on NetFX.

Which part is the problem?

[filipnavara]

            AssertExtensions.Throws<ArgumentOutOfRangeException>("millisecondsTimeOutInterval", () =>
                ThreadPool.RegisterWaitForSingleObject(waitHandle, callback, null, (long)int.MaxValue + 1, true));

It is silently truncated in NetFX (reference source):

        [System.Security.SecuritySafeCritical]  // auto-generated
        [MethodImplAttribute(MethodImplOptions.NoInlining)] // Methods containing StackCrawlMark local var has to be marked non-inlineable
        public static RegisteredWaitHandle RegisterWaitForSingleObject(  // throws RegisterWaitException
            WaitHandle          waitObject,
            WaitOrTimerCallback callBack,
            Object                  state,
            long                    millisecondsTimeOutInterval,
            bool                executeOnlyOnce    // NOTE: we do not allow other options that allow the callback to be queued as an APC
        )
        {
            if (millisecondsTimeOutInterval < -1)
                throw new ArgumentOutOfRangeException("millisecondsTimeOutInterval", Environment.GetResourceString("ArgumentOutOfRange_NeedNonNegOrNegative1"));
            Contract.EndContractBlock();
            StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
            return RegisterWaitForSingleObject(waitObject,callBack,state,(UInt32)millisecondsTimeOutInterval,executeOnlyOnce,ref stackMark,true);

[stephentoub]

Ok, then you can just surround that assert with:

if (!PlatformDetection.IsFullFramework) // netfx silently overflows the timeout
{
    ...
}

[filipnavara]

@dotnet-bot test UWP CoreCLR x64 Debug Build

[stephentoub]

Thanks