Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit d52dcd3

Browse files
committed
HttpClient xplat: Fix multiple issues with socket/timer callbacks
In earlier version, a curl_multi_socket_action was performed everytime the callback was invoked irrespective of whether there was activity on the socket. This in turn causes more callbacks when libcurl tries to read the socket but finds no data. This can be made more efficient by waiting for activity on the socket and informing libcurl accordingly - Fixed implementation of timer callback using an actual timer and handling all possible values of timeout - Fixed a bug in CheckForCompletedTransfers where a mismatch could occur between the easy handle specified in the DONE msg - Fixed the SEGV which was happening in earlier code due to recursive calls to curl_multi_socket_action in the socket callback which was being invoked synchronously in curl_multi_socket_action. This was causing a stack overflow leading to a segfault
1 parent c87771a commit d52dcd3

File tree

9 files changed

+480
-59
lines changed

9 files changed

+480
-59
lines changed

src/Common/src/Interop/Unix/libc/Interop.poll.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ internal static partial class libc
1212
internal enum PollFlags : short
1313
{
1414
POLLIN = 0x0001, /* any readable data available */
15+
POLLOUT = 0x0004, /* data can be written without blocking */
1516
POLLERR = 0x0008, /* some poll error occurred */
1617
POLLHUP = 0x0010, /* file descriptor was "hung up" */
1718
POLLNVAL = 0x0020, /* requested events "invalid" */
@@ -36,7 +37,7 @@ internal struct pollfd
3637
/// descriptors were ready, or -1 on error.
3738
/// </returns>
3839
[DllImport(Libraries.Libc, SetLastError = true)]
39-
private static unsafe extern int poll(pollfd* fds, uint count, int timeout);
40+
internal static unsafe extern int poll(pollfd* fds, uint count, int timeout);
4041

4142
/// <summary>
4243
/// Polls a File Descriptor for the passed in flags.

src/Common/src/Interop/Unix/libcurl/Interop.SafeCurlHandle.cs

Lines changed: 201 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,15 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System;
5+
using System.Collections.Generic;
6+
using System.Diagnostics;
57
using System.Runtime.InteropServices;
8+
using System.Threading;
9+
10+
using size_t = System.IntPtr;
11+
using libc = Interop.libc;
12+
using libcurl = Interop.libcurl;
13+
using PollFlags = Interop.libc.PollFlags;
614

715
internal static partial class Interop
816
{
@@ -30,16 +38,48 @@ public static void DisposeAndClearHandle(ref SafeCurlHandle curlHandle)
3038

3139
protected override bool ReleaseHandle()
3240
{
33-
Interop.libcurl.curl_easy_cleanup(this.handle);
41+
libcurl.curl_easy_cleanup(this.handle);
3442
return true;
3543
}
3644
}
3745

3846
internal sealed class SafeCurlMultiHandle : SafeHandle
3947
{
48+
private bool _pollCancelled = true;
49+
private readonly int[] _specialFds = new int[2];
50+
private readonly HashSet<int> _fdSet = new HashSet<int>();
51+
private int _requestCount = 0;
52+
private Timer _timer;
53+
54+
internal bool PollCancelled
55+
{
56+
get { return _pollCancelled; }
57+
set { _pollCancelled = value; }
58+
}
59+
60+
internal int RequestCount
61+
{
62+
get { return _requestCount; }
63+
set { _requestCount = value; }
64+
}
65+
66+
internal Timer Timer
67+
{
68+
get { return _timer; }
69+
set { _timer = value; }
70+
}
71+
72+
4073
public SafeCurlMultiHandle()
4174
: base(IntPtr.Zero, true)
4275
{
76+
unsafe
77+
{
78+
fixed(int* fds = _specialFds)
79+
{
80+
while (Interop.CheckIo(libc.pipe(fds)));
81+
}
82+
}
4383
}
4484

4585
public override bool IsInvalid
@@ -56,11 +96,169 @@ public static void DisposeAndClearHandle(ref SafeCurlMultiHandle curlHandle)
5696
}
5797
}
5898

99+
internal void PollFds(List<libc.pollfd> readyFds)
100+
{
101+
int count;
102+
libc.pollfd[] pollFds;
103+
104+
readyFds.Clear();
105+
106+
lock (this)
107+
{
108+
// TODO: Avoid the allocation when count is in 100s
109+
count = _fdSet.Count + 1;
110+
pollFds = new libc.pollfd[count];
111+
112+
// Always include special fd in the poll set. This is used to
113+
// return from the poll in case any fds have been added or
114+
// removed to the set of fds being polled. This prevents starvation
115+
// in case current set of fds have no activity but the new fd
116+
// is ready for a read/write. The special fd is the read end of a pipe
117+
// Whenever an fd is added/removed in _fdSet, a write happens to the
118+
// write end of the pipe thus causing the poll to return.
119+
pollFds[0].fd = _specialFds[libc.ReadEndOfPipe];
120+
pollFds[0].events = PollFlags.POLLIN | PollFlags.POLLOUT;
121+
int i = 1;
122+
foreach (int fd in _fdSet)
123+
{
124+
pollFds[i].fd = fd;
125+
pollFds[i].events = PollFlags.POLLIN | PollFlags.POLLOUT;
126+
i++;
127+
}
128+
}
129+
130+
unsafe
131+
{
132+
fixed (libc.pollfd* fds = pollFds)
133+
{
134+
int numFds = libc.poll(fds, (uint)count, -1);
135+
if (numFds <= 0)
136+
{
137+
Debug.Assert(numFds != 0); // Since timeout is infinite
138+
139+
// TODO: How to handle errors?
140+
throw new InvalidOperationException("Poll failure: " + Marshal.GetLastWin32Error());
141+
}
142+
143+
lock (this)
144+
{
145+
if (0 == _requestCount)
146+
{
147+
return;
148+
}
149+
}
150+
151+
// Check for any fdset changes
152+
if (fds[0].revents != 0)
153+
{
154+
if (ReadSpecialFd(fds[0].revents) < 0)
155+
{
156+
// TODO: How to handle errors?
157+
throw new InvalidOperationException("Cannot read data: " + Marshal.GetLastWin32Error());
158+
}
159+
numFds--;
160+
}
161+
162+
// Now check for events on the remaining fds
163+
for (int i = 1; i < count && numFds > 0; i++)
164+
{
165+
if (fds[i].revents == 0)
166+
{
167+
continue;
168+
}
169+
readyFds.Add(fds[i]);
170+
numFds--;
171+
}
172+
}
173+
}
174+
}
175+
176+
internal void SignalFdSetChange(int fd, bool isRemove)
177+
{
178+
Debug.Assert(Monitor.IsEntered(this));
179+
180+
bool changed = isRemove ? _fdSet.Remove(fd) : _fdSet.Add(fd);
181+
if (!changed)
182+
{
183+
return;
184+
}
185+
186+
unsafe
187+
{
188+
// Write to special fd
189+
byte* dummyBytes = stackalloc byte[1];
190+
if ((int)libc.write(_specialFds[libc.WriteEndOfPipe], dummyBytes, (size_t)1) <= 0)
191+
{
192+
// TODO: How to handle errors?
193+
throw new InvalidOperationException("Cannot write data: " + Marshal.GetLastWin32Error());
194+
}
195+
}
196+
}
197+
198+
protected override void Dispose(bool disposing)
199+
{
200+
if (disposing)
201+
{
202+
if (null != _timer)
203+
{
204+
_timer.Dispose();
205+
}
206+
}
207+
base.Dispose(disposing);
208+
}
209+
59210
protected override bool ReleaseHandle()
60211
{
61-
Interop.libcurl.curl_multi_cleanup(this.handle);
212+
Debug.Assert(0 == _fdSet.Count);
213+
Debug.Assert(0 == _requestCount);
214+
Debug.Assert(_pollCancelled);
215+
216+
libc.close(_specialFds[libc.ReadEndOfPipe]);
217+
libc.close(_specialFds[libc.WriteEndOfPipe]);
218+
libcurl.curl_multi_cleanup(this.handle);
219+
62220
return true;
63221
}
222+
223+
private int ReadSpecialFd(PollFlags revents)
224+
{
225+
PollFlags badEvents = PollFlags.POLLERR | PollFlags.POLLHUP | PollFlags.POLLNVAL;
226+
if ((revents & badEvents) != 0)
227+
{
228+
return -1;
229+
}
230+
int pipeReadFd = _specialFds[libc.ReadEndOfPipe];
231+
int bytesRead = 0;
232+
unsafe
233+
{
234+
do
235+
{
236+
// Read available data from the pipe
237+
int bufferLength = 1024;
238+
byte* dummyBytes = stackalloc byte[bufferLength];
239+
int numBytes = (int)libc.read(pipeReadFd, dummyBytes, (size_t)bufferLength);
240+
if (numBytes <= 0)
241+
{
242+
return -1;
243+
}
244+
bytesRead += numBytes;
245+
246+
// Check if more data is available
247+
PollFlags outFlags;
248+
int retVal = libc.poll(pipeReadFd, PollFlags.POLLIN, 0, out outFlags);
249+
if (retVal < 0)
250+
{
251+
return -1;
252+
}
253+
else if (0 == retVal)
254+
{
255+
break;
256+
}
257+
}
258+
while (true);
259+
}
260+
return bytesRead;
261+
}
64262
}
65263

66264
internal sealed class SafeCurlSlistHandle : SafeHandle
@@ -91,7 +289,7 @@ public static void DisposeAndClearHandle(ref SafeCurlSlistHandle curlHandle)
91289

92290
protected override bool ReleaseHandle()
93291
{
94-
Interop.libcurl.curl_slist_free_all(this.handle);
292+
libcurl.curl_slist_free_all(this.handle);
95293
return true;
96294
}
97295
}

src/System.Net.Http/src/Resources/Strings.resx

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,4 +294,37 @@
294294
<data name="net_http_unix_https_support_unavailable_libcurl" xml:space="preserve">
295295
<value>libcurl library available on the system does not support https</value>
296296
</data>
297+
<data name="ArgumentOutOfRange_FileLengthTooBig" xml:space="preserve">
298+
<value>Specified file length was too large for the file system.</value>
299+
</data>
300+
<data name="IO_FileExists_Name" xml:space="preserve">
301+
<value>The file '{0}' already exists.</value>
302+
</data>
303+
<data name="IO_FileNotFound" xml:space="preserve">
304+
<value>Unable to find the specified file.</value>
305+
</data>
306+
<data name="IO_FileNotFound_FileName" xml:space="preserve">
307+
<value>Could not find file '{0}'.</value>
308+
</data>
309+
<data name="IO_PathNotFound_NoPathName" xml:space="preserve">
310+
<value>Could not find a part of the path.</value>
311+
</data>
312+
<data name="IO_PathNotFound_Path" xml:space="preserve">
313+
<value>Could not find a part of the path '{0}'.</value>
314+
</data>
315+
<data name="IO_PathTooLong" xml:space="preserve">
316+
<value>The specified file name or path is too long, or a component of the specified path is too long.</value>
317+
</data>
318+
<data name="IO_SharingViolation_File" xml:space="preserve">
319+
<value>The process cannot access the file '{0}' because it is being used by another process.</value>
320+
</data>
321+
<data name="IO_SharingViolation_NoFileName" xml:space="preserve">
322+
<value>The process cannot access the file because it is being used by another process.</value>
323+
</data>
324+
<data name="UnauthorizedAccess_IODenied_NoPathName" xml:space="preserve">
325+
<value>Access to the path is denied.</value>
326+
</data>
327+
<data name="UnauthorizedAccess_IODenied_Path" xml:space="preserve">
328+
<value>Access to the path '{0}' is denied.</value>
329+
</data>
297330
</root>

src/System.Net.Http/src/System.Net.Http.csproj

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,33 @@
140140
<Compile Include="System\Net\Http\Unix\CurlHandler.cs" />
141141
<Compile Include="System\Net\Http\Unix\CurlResponseMessage.cs" />
142142
<Compile Include="System\Net\Http\Unix\HttpClientHandler.Unix.cs" />
143+
<Compile Include="$(CommonPath)\Interop\Unix\Interop.Errors.cs">
144+
<Link>Common\Interop\Unix\Interop.Errors.cs</Link>
145+
</Compile>
146+
<Compile Include="$(CommonPath)\Interop\Unix\Interop.IOErrors.cs">
147+
<Link>Common\Interop\Unix\Interop.IOErrors.cs</Link>
148+
</Compile>
143149
<Compile Include="$(CommonPath)\Interop\Unix\Interop.Libraries.cs">
144150
<Link>Common\Interop\Unix\Interop.Libraries.cs</Link>
145151
</Compile>
152+
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.close.cs">
153+
<Link>Common\Interop\Unix\libc\Interop.close.cs</Link>
154+
</Compile>
155+
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.open.cs">
156+
<Link>Common\Interop\Unix\libc\Interop.open.cs</Link>
157+
</Compile>
158+
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.pipe.cs">
159+
<Link>Common\Interop\Unix\libc\Interop.pipe.cs</Link>
160+
</Compile>
161+
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.poll.cs">
162+
<Link>Common\Interop\Unix\libc\Interop.poll.cs</Link>
163+
</Compile>
164+
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.read.cs">
165+
<Link>Common\Interop\Unix\libc\Interop.read.cs</Link>
166+
</Compile>
167+
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.write.cs">
168+
<Link>Common\Interop\Unix\libc\Interop.write.cs</Link>
169+
</Compile>
146170
<Compile Include="$(CommonPath)\Interop\Unix\libcurl\Interop.libcurl.cs">
147171
<Link>Common\Interop\Unix\libcurl\Interop.libcurl.cs</Link>
148172
</Compile>
@@ -156,6 +180,21 @@
156180
<Link>Common\System\NotImplemented.cs</Link>
157181
</Compile>
158182
</ItemGroup>
183+
<ItemGroup Condition=" '$(TargetsLinux)' == 'true' ">
184+
<Compile Include="$(CommonPath)\Interop\Linux\libc\Interop.OpenFlags.cs">
185+
<Link>Common\Interop\Linux\libc\Interop.OpenFlags.cs</Link>
186+
</Compile>
187+
</ItemGroup>
188+
<ItemGroup Condition=" '$(TargetsOSX)' == 'true' ">
189+
<Compile Include="$(CommonPath)\Interop\OSX\libc\Interop.OpenFlags.cs">
190+
<Link>Common\Interop\OSX\libc\Interop.OpenFlags.cs</Link>
191+
</Compile>
192+
</ItemGroup>
193+
<ItemGroup Condition=" '$(TargetsFreeBSD)' == 'true' ">
194+
<Compile Include="$(CommonPath)\Interop\FreeBSD\libc\Interop.OpenFlags.cs">
195+
<Link>Common\Interop\FreeBSD\libc\Interop.OpenFlags.cs</Link>
196+
</Compile>
197+
</ItemGroup>
159198

160199
<ItemGroup>
161200
<None Include="project.json" />

0 commit comments

Comments
 (0)