Expose SocketsHttpHandler #26964

stephentoub · 2018-02-08T19:30:00Z

Renames ManagedHandler to SocketsHttpHandler
Moves a bunch of files around accordingly
Updates System.Net.WebSockets.Client to use SocketsHttpHandler directly
Updates System.Net.Http's tests with a reflection-based rather than TLS-based method for instantiating an HttpClientHandler backed by a SocketsHttpHandler.
Adds some more tests

cc: @geoffkizer, @davidsh, @Priya91, @wfurt, @karelz
Closes https://github.com/dotnet/corefx/issues/23166
Closes https://github.com/dotnet/corefx/issues/26895
Closes https://github.com/dotnet/corefx/issues/26960

davidsh · 2018-02-08T19:51:05Z

src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs

+        protected override bool UseSocketsHttpHandler => true;
+
+        // TODO: Currently the subsequent tests sometimes fail/hang with WinHttpHandler / CurlHandler.
+        // In theory they should pass with any handler that does appropriate connection pooling.


Is there an issue number for this TODO?

Probably not. I'll open one.

pjanotti · 2018-02-08T20:50:10Z

src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs

+
+                            // Make second request and expect it to be served from a different connection.
+                            Task<string> request2 = client.GetStringAsync(uri);
+                            await LoopbackServer.AcceptSocketAsync(listener, async (server2, serverStream2, serverReader2, serverWriter2) =>


AcceptSocketAsync [](start = 49, length = 17)

Q: this is a pattern in most of the tests, but in case of client/handler bug the call to AcceptSocketAsync can cause the test to hang. Is there any reason to not add a timeout on the AcceptAsync() call on LoopbackServer.AcceptSocketAsync?

We can do that, we would just want it to be really long, like a minute.

pjanotti · 2018-02-08T20:54:05Z

src/System.Net.Http/tests/FunctionalTests/HttpClientTestBase.cs

+        protected bool IsWinHttpHandler => !UseSocketsHttpHandler && PlatformDetection.IsWindows && !PlatformDetection.IsUap && !PlatformDetection.IsFullFramework;
+        protected bool IsCurlHandler => !UseSocketsHttpHandler && !PlatformDetection.IsWindows;
+        protected bool IsNetfxHandler => !UseSocketsHttpHandler && PlatformDetection.IsWindows && PlatformDetection.IsFullFramework;
+        protected bool IsUapHandler => !UseSocketsHttpHandler && PlatformDetection.IsWindows && PlatformDetection.IsUap;


pjanotti

LGTM. I didn't know that a rename was coming, it is welcome.

geoffkizer · 2018-02-08T21:38:21Z

LGTM

stephentoub · 2018-02-08T22:43:43Z

@dotnet-bot test Windows x64 Debug Build please

- Renames ManagedHandler to SocketsHttpHandler - Moves a bunch of files around accordingly - Updates System.Net.WebSockets.Client to use SocketsHttpHandler directly - Updates System.Net.Http's tests with a reflection-based rather than TLS-based method for instantiating an HttpClientHandler backed by a SocketsHttpHandler.

stephentoub · 2018-02-09T01:36:17Z

@dotnet/dnceng, my legs are failing with errors like this:

Using context: Windows x86 Release Build
java.lang.NullPointerException
at java.util.Collections$UnmodifiableCollection.(Collections.java:1026)
at java.util.Collections$UnmodifiableList.(Collections.java:1302)
at java.util.Collections.unmodifiableList(Collections.java:1287)
at hudson.model.ParametersAction.getAllParameters(ParametersAction.java:351)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.jenkinsci.plugins.workflow.cps.ParamsVariable.getValue(ParamsVariable.java:61)
at org.jenkinsci.plugins.workflow.cps.CpsScript.getProperty(CpsScript.java:121)
at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:174)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.getProperty(ScriptBytecodeAdapter.java:456)
at org.kohsuke.groovy.sandbox.impl.Checker$6.call(Checker.java:284)
at org.kohsuke.groovy.sandbox.GroovyInterceptor.onGetProperty(GroovyInterceptor.java:68)
at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.onGetProperty(SandboxInterceptor.java:326)
at org.kohsuke.groovy.sandbox.impl.Checker$6.call(Checker.java:282)
at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:286)
at com.cloudbees.groovy.cps.sandbox.SandboxInvoker.getProperty(SandboxInvoker.java:29)
at com.cloudbees.groovy.cps.impl.PropertyAccessBlock.rawGet(PropertyAccessBlock.java:20)
Caused: java.lang.reflect.InvocationTargetException
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.jenkinsci.plugins.workflow.cps.ParamsVariable.getValue(ParamsVariable.java:61)
at org.jenkinsci.plugins.workflow.cps.CpsScript.getProperty(CpsScript.java:121)
at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:174)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.getProperty(ScriptBytecodeAdapter.java:456)
at org.kohsuke.groovy.sandbox.impl.Checker$6.call(Checker.java:284)
at org.kohsuke.groovy.sandbox.GroovyInterceptor.onGetProperty(GroovyInterceptor.java:68)
at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.onGetProperty(SandboxInterceptor.java:326)
at org.kohsuke.groovy.sandbox.impl.Checker$6.call(Checker.java:282)
at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:286)
at com.cloudbees.groovy.cps.sandbox.SandboxInvoker.getProperty(SandboxInvoker.java:29)
at com.cloudbees.groovy.cps.impl.PropertyAccessBlock.rawGet(PropertyAccessBlock.java:20)
at WorkflowScript.run(WorkflowScript:11)
at cps.transform(Native Method)
at com.cloudbees.groovy.cps.impl.PropertyishBlock$ContinuationImpl.get(PropertyishBlock.java:74)
at com.cloudbees.groovy.cps.LValueBlock$GetAdapter.receive(LValueBlock.java:30)
at com.cloudbees.groovy.cps.impl.PropertyishBlock$ContinuationImpl.fixName(PropertyishBlock.java:66)
at sun.reflect.GeneratedMethodAccessor187.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at com.cloudbees.groovy.cps.impl.ContinuationPtr$ContinuationImpl.receive(ContinuationPtr.java:72)
at com.cloudbees.groovy.cps.impl.ConstantBlock.eval(ConstantBlock.java:21)
at com.cloudbees.groovy.cps.Next.step(Next.java:83)
at com.cloudbees.groovy.cps.Continuable$1.call(Continuable.java:174)
at com.cloudbees.groovy.cps.Continuable$1.call(Continuable.java:163)
at org.codehaus.groovy.runtime.GroovyCategorySupport$ThreadCategoryInfo.use(GroovyCategorySupport.java:122)
at org.codehaus.groovy.runtime.GroovyCategorySupport.use(GroovyCategorySupport.java:261)
at com.cloudbees.groovy.cps.Continuable.run0(Continuable.java:163)
at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.access$001(SandboxContinuable.java:19)
at org.jenkinsci.plugins.workflow.cps.SandboxContinuable$1.call(SandboxContinuable.java:35)
at org.jenkinsci.plugins.workflow.cps.SandboxContinuable$1.call(SandboxContinuable.java:32)
at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox.runInSandbox(GroovySandbox.java:108)
at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.run0(SandboxContinuable.java:32)
at org.jenkinsci.plugins.workflow.cps.CpsThread.runNextChunk(CpsThread.java:174)
at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.run(CpsThreadGroup.java:330)
at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.access$100(CpsThreadGroup.java:82)
at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:242)
at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:230)
at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$2.call(CpsVmExecutorService.java:64)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:112)
at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
Finished: FAILURE

MattGal · 2018-02-09T01:48:09Z

@stephentoub : @riarenas was working on this and ended up rebooting ci3.dot.net. Let's try a retry first?

@dotnet-bot test please

stephentoub · 2018-02-09T02:45:05Z

@dotnet-bot test this please

…handler Expose SocketsHttpHandler Commit migrated from dotnet/corefx@eb89247

davidsh added the area-System.Net.Http.ManagedHandler label Feb 8, 2018

davidsh reviewed Feb 8, 2018

View reviewed changes

stephentoub force-pushed the exposemanagedhandler branch from 57cb2d8 to 578d6c5 Compare February 8, 2018 20:13

pjanotti reviewed Feb 8, 2018

View reviewed changes

pjanotti approved these changes Feb 8, 2018

View reviewed changes

davidsh approved these changes Feb 8, 2018

View reviewed changes

stephentoub added 3 commits February 8, 2018 19:12

Add more SocketsHttpHandler tests

8cb4d13

Fix WebSocketProtocol test that never worked on netfx

f6f5188

stephentoub force-pushed the exposemanagedhandler branch from 54dc1be to f6f5188 Compare February 9, 2018 00:13

karelz assigned stephentoub Feb 9, 2018

stephentoub merged commit eb89247 into dotnet:master Feb 9, 2018

stephentoub deleted the exposemanagedhandler branch February 9, 2018 04:04

karelz added this to the 2.1.0 milestone Mar 10, 2018

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Merge pull request dotnet/corefx#26964 from stephentoub/exposemanaged…

2fbfd02

…handler Expose SocketsHttpHandler Commit migrated from dotnet/corefx@eb89247

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose SocketsHttpHandler #26964

Expose SocketsHttpHandler #26964

stephentoub commented Feb 8, 2018 •

edited

Loading

davidsh Feb 8, 2018

stephentoub Feb 8, 2018

pjanotti Feb 8, 2018

stephentoub Feb 8, 2018

pjanotti Feb 8, 2018

pjanotti left a comment

geoffkizer commented Feb 8, 2018

stephentoub commented Feb 8, 2018

stephentoub commented Feb 9, 2018

MattGal commented Feb 9, 2018

stephentoub commented Feb 9, 2018

Expose SocketsHttpHandler #26964

Expose SocketsHttpHandler #26964

Conversation

stephentoub commented Feb 8, 2018 • edited Loading

davidsh Feb 8, 2018

Choose a reason for hiding this comment

stephentoub Feb 8, 2018

Choose a reason for hiding this comment

pjanotti Feb 8, 2018

Choose a reason for hiding this comment

stephentoub Feb 8, 2018

Choose a reason for hiding this comment

pjanotti Feb 8, 2018

Choose a reason for hiding this comment

pjanotti left a comment

Choose a reason for hiding this comment

geoffkizer commented Feb 8, 2018

stephentoub commented Feb 8, 2018

stephentoub commented Feb 9, 2018

MattGal commented Feb 9, 2018

stephentoub commented Feb 9, 2018

stephentoub commented Feb 8, 2018 •

edited

Loading