Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

fixes for ARM32 #34559

Merged
merged 2 commits into from
Jan 18, 2019
Merged

fixes for ARM32 #34559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fe4f3f
arm fixes
wfurt Jan 11, 2019
4441d5e
use PassingTestTimeoutMilliseconds instead of hardcoded number
wfurt Jan 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/System.Net.Http/src/System/Net/Http/CurlHandler/CurlHandler.EasyRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ private void SetMultithreading()

private void SetTimeouts()
{
// Set timeout limit on the connect phase.
SetCurlOption(CURLoption.CURLOPT_CONNECTTIMEOUT_MS, int.MaxValue);
// Set timeout limit on the connect phase. curl has bug on ARM so use max - 1s.
SetCurlOption(CURLoption.CURLOPT_CONNECTTIMEOUT_MS, int.MaxValue - 1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth opening a bug against curl, even if we have a work around. Their bug tracker is here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about it. I would need to probably isolate C repo and show behavior with some public server.


// Override the default DNS cache timeout. libcurl defaults to a 1 minute
// timeout, but we extend that to match the Windows timeout of 10 minutes.
Expand Down
10 changes: 9 additions & 1 deletion src/System.Net.Http/tests/FunctionalTests/NtAuthTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,21 @@ public NtAuthServer(AuthenticationSchemes authSchemes)
{
// Ignore.
}
catch (ObjectDisposedException)
{
// Ignore.
}
catch (InvalidOperationException)
{
// Ignore.
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it's hiding a symptom. Is there an underlying problem we should be fixing instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be wrong but I don't think so. We initialize instances when the class loads. And then we skip the test and we immediately hit Dispose() Now, on busy/slow system this happens before the task runs end executes the listener call. That can either lead to timeout, object disposed exception or invalid operation based on exact timing. In that case I saw on ARM32 the actual test was skipped but hit "Error: (not Fail) just loading and unloading the test class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on busy/slow system this happens before the task runs end executes the listener call

What's the stack of the exceptions you're seeing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from my x64 VM Machine

       Condition(s) not met: "IsSelectedSitesTestEnabled"
      [Test Class Cleanup Failure (System.Net.Http.Functional.Tests.NtAuthTests)] System.AggregateException
        System.AggregateException : One or more errors occurred. (Cannot access a disposed object.
        Object name: 'listener'.)
        ---- System.ObjectDisposedException : Cannot access a disposed object.
        Object name: 'listener'.
        Stack Trace:
          /root/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs(2843,0): at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
          /root/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs(2714,0): at System.Threading.Tasks.Task.Wait()
          /home/furt/git/wfurt-corefx/src/System.Net.Http/tests/FunctionalTests/NtAuthTests.cs(57,0): at System.Net.Http.Functional.Tests.NtAuthServer.Dispose()
          /home/furt/git/wfurt-corefx/src/System.Net.Http/tests/FunctionalTests/NtAuthTests.cs(95,0): at System.Net.Http.Functional.Tests.NtAuthServers.Dispose()
          ----- Inner Stack Trace -----
          /home/furt/git/wfurt-corefx/src/System.Net.HttpListener/src/System/Net/Managed/ListenerAsyncResult.Managed.cs(197,0): at System.Net.ListenerAsyncResult.GetContext()
          /home/furt/git/wfurt-corefx/src/System.Net.HttpListener/src/System/Net/Managed/HttpListener.Managed.cs(347,0): at System.Net.HttpListener.EndGetContext(IAsyncResult asyncResult)
          /home/furt/git/wfurt-corefx/src/System.Net.HttpListener/src/System/Net/HttpListener.cs(290,0): at System.Net.HttpListener.<>c.<GetContextAsync>b__44_1(IAsyncResult iar)
          /root/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/FutureFactory.cs(529,0): at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
          --- End of stack trace from previous location where exception was thrown ---
          /home/furt/git/wfurt-corefx/src/System.Net.Http/tests/FunctionalTests/NtAuthTests.cs(34,0): at System.Net.Http.Functional.Tests.NtAuthServer.<.ctor>b__4_0()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, ok. GetContext documents both InvalidOperationException and ObjectDisposedException as occurring in cases like this, where the HttpListener may not be listening or has been disposed of already, so... ok. Subsequently it might be nice to try to restructure the tests to avoid this, but I agree your tactical fix is good to go ahead with now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally started on path to add synchronization.
But than it looked like unnecessary complexity for cases when we do not run the test anyway.

});
}

public void Dispose()
{
_listener.Stop();
_serverTask.Wait();
_serverTask.Wait(TestHelper.PassingTestTimeoutMilliseconds);
}

public string BaseUrl { get; private set; }
Expand Down
2 changes: 1 addition & 1 deletion src/System.Net.Http/tests/FunctionalTests/TestHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public static byte[] ComputeMD5Hash(byte[] data)

public static Task WhenAllCompletedOrAnyFailed(params Task[] tasks)
{
return TaskTimeoutExtensions.WhenAllOrAnyFailed(tasks);
return TaskTimeoutExtensions.WhenAllOrAnyFailed(tasks, PlatformDetection.IsArmProcess || PlatformDetection.IsArm64Process ? PassingTestTimeoutMilliseconds * 5 : PassingTestTimeoutMilliseconds);
}

public static Task WhenAllCompletedOrAnyFailedWithTimeout(int timeoutInMilliseconds, params Task[] tasks)
Expand Down