Bug 43 - WebRequests via ThreadPool fail and hand, single thread does not
Summary: WebRequests via ThreadPool fail and hand, single thread does not
Status: RESOLVED NOT_ON_ROADMAP
Alias: None
Product: Class Libraries
Classification: Mono
Component: System ()
Version: 2.10.x
Hardware: PC Linux
: --- normal
Target Milestone: Untriaged
Assignee: Gonzalo Paniagua Javier
URL:
Depends on:
Blocks:
 
Reported: 2011-07-22 10:28 UTC by Chris Morgan
Modified: 2011-08-11 17:28 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Test Program (2.52 KB, text/x-csharp)
2011-07-22 10:28 UTC, Chris Morgan
Details
Modified test case - added 'awesome' command line option (3.66 KB, text/plain)
2011-08-11 14:12 UTC, Gonzalo Paniagua Javier
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED NOT_ON_ROADMAP

Description Chris Morgan 2011-07-22 10:28:06 UTC
Created attachment 10 [details]
Test Program

Repeated WebRequests via a ThreadPool eventually fail due to request timeouts
(after hundreds) and continue to fail once the first failure happens.

The same requests from a single thread continue indefinitely (10k+ requests).


Reproducible: Always

Steps to Reproduce:
1.Issue WebRequests from ThreadPool via a WaitCallback()
2.See timeout exceptions


Expected Results:  
Continuous WebRequests without timeout exceptions.

Attached a test program that when run with 'pool' will fail after several runs
and when run with 'single' will run indefinitely.

Also tested with mono 2.6.7 and the git version of mono with similar or worse
results.
Comment 1 Miguel de Icaza [MSFT] 2011-07-22 11:31:33 UTC
Threadpools do not interact well with asynchronous operations, which WebRequest uses internally, you might have to throttle:

http://www.mono-project.com/ThreadPool_DeadLocks

You can work around this by increasing the number of threads allowed in the threadpool.

Gonzalo, did I get that right?
Comment 2 Gonzalo Paniagua Javier 2011-07-22 11:50:36 UTC
Miguel, the thereadpools (are supposed to) interact well with asynchronous operations. Throttling is done internally in HttpWebRequest (configure system.net/connectionManagement) and the page you mention is obsolete since ~2.8.

The problem happens in 2.6.7 "also", but what version is Chris using as the main one? This should not be an issue with 2.8.x (latest) and 2.10.x (not on OSX, wait for 2.10.3 in that case).
Comment 3 Chris Morgan 2011-07-22 11:54:54 UTC
I tried with the head of git master last week and it worked even worse than 2.6.7 does (Ubuntu 11.04).

The oddity is that once a first request fails no future requests will work.
Comment 4 Gonzalo Paniagua Javier 2011-07-23 14:14:15 UTC
I'll try to repro with your test case in master and will see what I can do.
Comment 5 Ethan Nagel 2011-07-25 19:39:46 UTC
We had this problem with a service we built and eventually began using the curl library.   Would love to use built-in WebRequests instead once this is reliable.
Comment 6 Gonzalo Paniagua Javier 2011-08-11 14:11:29 UTC
Chris,

If you run the test I will attach after this commen and pass in the option 'awesome' you'll see that the test will work.

When you pass 'pool', under MS .NET the number of worker threads never stops growing (I stopped the test at 850) and with synchronous requests, everything works fine.

What happens with 'pool'? You are blocking in a threadpool thread by doing a synchronous call to GetResponse (). Every time RetrieveEntry() is called it will send something new to the threadpool and sit there until the request is finished. And so do all the other threads that keep piling up. Unfortunately, Mono fails in a different way than .NET because HttpWebRequest generates internal threadpool work items itself until it gets to a socket asynchronous call.

If you want to do retrieve a lot of URLs asynchrnously, use the Begin/End interface. And if you need timeouts for these, just add a Timer that will execute a webRequest.Abort () if nothing happens within your desired time frame.
Comment 7 Gonzalo Paniagua Javier 2011-08-11 14:12:42 UTC
Created attachment 112 [details]
Modified test case - added 'awesome' command line option
Comment 8 Chris Morgan 2011-08-11 16:18:56 UTC
Hi Gonzalo.

So the issue is that the call to GetResponse() is itself adding threads to the threadpool which is already occupied with the many other requests that have been made? So this means that the synchronous GetResponse() is itself blocked and blocking a thread in the pool because another threadpool request is waiting to be processed?

Can't help but think that the change is simply working around the issue and that because the internal requests themselves go into the same pool that this kind of situation will be hit via other kinds of pooled operations and be just as confusing for other users.

.Net keeps creating worker threads forever? Or until memory runs out? Imo it seems like any internally queued requests should have priority over externally queued ones, or a slot should remain free for them or they should be executed without themselves queuing workitems. Thoughts?

Chris
Comment 9 Gonzalo Paniagua Javier 2011-08-11 16:36:06 UTC
>So the issue is that the call to GetResponse() is itself adding threads to the
>threadpool which is already occupied with the many other requests that have
>been made? So this means that the synchronous GetResponse() is itself blocked
>and blocking a thread in the pool because another threadpool request is waiting
>to be processed?

GetResponse() does not add threads to the threadpool but work items, which might or might not create a new thread. And, yes, the synchronous call to GetResponse() is using the threadpool thread running it entirely for itself.

>Can't help but think that the change is simply working around the issue and
>that because the internal requests themselves go into the same pool that this
>kind of situation will be hit via other kinds of pooled operations and be just
>as confusing for other users.

Your test is misusing the threadpool. The code I added is using it right. Threadpool threads are not supposed to run code that blocks or that takes a long time to run.

> .Net keeps creating worker threads forever? Or until memory runs out? Imo it
> seems like any internally queued requests should have priority over externally
> queued ones, or a slot should remain free for them or they should be executed
> without themselves queuing workitems. Thoughts?

"Forever" I assume. I have a lot of RAM in my machine and didn't want to wait until it run out of resources.

Internally queued requests use the same interface as the external ones :-(

What we could do much better in HttpWebRequest is to use the socket asynchronous 
 method in a better way and avoid creating more work items as much as possible. Right now, I think that the worse part is that we call Connect() synchronously (which is the same problem that happens with your test, only that the max number of connections is limited).

Another option that is available if you want to keep the code as it is is the use of a different threadpool. I've used the SmartThreadPool in the past and it worked great.
Comment 10 Chris Morgan 2011-08-11 17:02:41 UTC
(In reply to comment #9)
> >So the issue is that the call to GetResponse() is itself adding threads to the
> >threadpool which is already occupied with the many other requests that have
> >been made? So this means that the synchronous GetResponse() is itself blocked
> >and blocking a thread in the pool because another threadpool request is waiting
> >to be processed?
> 
> GetResponse() does not add threads to the threadpool but work items, which
> might or might not create a new thread. And, yes, the synchronous call to
> GetResponse() is using the threadpool thread running it entirely for itself.
> 
> >Can't help but think that the change is simply working around the issue and
> >that because the internal requests themselves go into the same pool that this
> >kind of situation will be hit via other kinds of pooled operations and be just
> >as confusing for other users.
> 
> Your test is misusing the threadpool. The code I added is using it right.
> Threadpool threads are not supposed to run code that blocks or that takes a
> long time to run.
>

I think maybe misusing is a bit of a strong term. No mention here, http://msdn.microsoft.com/en-us/library/3dasc8as(v=vs.80).aspx, about short lived tasks that don't block.



> > .Net keeps creating worker threads forever? Or until memory runs out? Imo it
> > seems like any internally queued requests should have priority over externally
> > queued ones, or a slot should remain free for them or they should be executed
> > without themselves queuing workitems. Thoughts?
> 
> "Forever" I assume. I have a lot of RAM in my machine and didn't want to wait
> until it run out of resources.
> 


And the requests are being processed? Really if the internal requests chained to a specific ThreadPool workitem were somehow prioritized then maybe the system would be self correcting. I would imagine proper behavior would look something like:

- Workitems added to ThreadPool until memory runs out (I don't recall anything from ThreadPool docs that suggests a limit to queued Workitems but maybe there is and that limit would be hit first)
- Workitems would be processed by available threads and any Workitems created from processing wouldn't be held back by a full pool.

Doesn't this seem like the desired behavior? I mean maybe it isn't easy to do but we should look first at what we'd like to achieve.


> Internally queued requests use the same interface as the external ones :-(
> 
> What we could do much better in HttpWebRequest is to use the socket
> asynchronous 
>  method in a better way and avoid creating more work items as much as possible.
> Right now, I think that the worse part is that we call Connect() synchronously
> (which is the same problem that happens with your test, only that the max
> number of connections is limited).
> 


Changing this might help the issue but if there were any internally queued requests I think we'd have the same issue.

I'd be open to helping with the solution here in terms of development.


> Another option that is available if you want to keep the code as it is is the
> use of a different threadpool. I've used the SmartThreadPool in the past and it
> worked great.

This wouldn't help other users of the Mono threadpool that might run into similar issues.
Comment 11 Gonzalo Paniagua Javier 2011-08-11 17:20:29 UTC
>I think maybe misusing is a bit of a strong term. No mention here,
>http://msdn.microsoft.com/en-us/library/3dasc8as(v=vs.80).aspx, about short
>lived tasks that don't block.

Yeah, I should have quoted it or use a different word.

The problem is not queuing the work items. The problem is the dependency between them. If at some point the thread pool work items stop competing with the items you add to the threadpool, chances are that the test will work. For instance, I just tried SetMinThreads (100, 1) and limited the loop to 100 times and it worked.

>- Workitems added to ThreadPool until memory runs out (I don't recall anything
>from ThreadPool docs that suggests a limit to queued Workitems but maybe there
>is and that limit would be hit first)

If you have a queue of 8GB, GC will be your real problem.

>- Workitems would be processed by available threads and any Workitems created
>from processing wouldn't be held back by a full pool.

Workitems *are* processed by available threads, if there's any. If not, a new thread is created every 0.5s, so your 10s timeout is too short.
Comment 12 Chris Morgan 2011-08-11 17:28:26 UTC
(In reply to comment #11)
> >I think maybe misusing is a bit of a strong term. No mention here,
> >http://msdn.microsoft.com/en-us/library/3dasc8as(v=vs.80).aspx, about short
> >lived tasks that don't block.
> 
> Yeah, I should have quoted it or use a different word.
> 
> The problem is not queuing the work items. The problem is the dependency
> between them. If at some point the thread pool work items stop competing with
> the items you add to the threadpool, chances are that the test will work. For
> instance, I just tried SetMinThreads (100, 1) and limited the loop to 100 times
> and it worked.
>


I'd rather it worked like .Net did and processed more entries at the expense of running my system out of memory. Right now it hits a wall and never seems to recover even after several minutes of waiting. Consider also that if the earliest Workitems aren't being processed the connections to the server might be being closed.

> >- Workitems added to ThreadPool until memory runs out (I don't recall anything
> >from ThreadPool docs that suggests a limit to queued Workitems but maybe there
> >is and that limit would be hit first)
> 
> If you have a queue of 8GB, GC will be your real problem.
> 
> >- Workitems would be processed by available threads and any Workitems created
> >from processing wouldn't be held back by a full pool.
> 
> Workitems *are* processed by available threads, if there's any. If not, a new
> thread is created every 0.5s, so your 10s timeout is too short.

I can't help but think there is a better way. I don't know the design at all now though. If you can think of one that I could try out please let me know and I'll try to help out.