8

So after dabbling with Java and HttpClient, I've decided to transition to C# to try and lower the memory usage while increasing speed. I've been reading tons of articles provided by members on here regarding async vs multithreading. It appears multithreading would be the better direction.

My program will be accessing a server and sending the same requests over and over until a 200 code is retrieved. Reason being is because during peak hours the traffic is very high. It makes the server is VERY hard to reach and throws 5xx errors. The code is very bare, I didn't want to add the loop yet to help with simplicity for the viewers.

I feel that I am heading towards the right direction but I would like to reach out to the community to break out of any bad habits. Thanks again for reading.

Note to moderators: I am asking to check my code for discrepancies, I am well aware that it's okay to multithread HttpClient.

using System;
using System.Net.Http;
using System.Collections.Generic;
using System.Net.Http.Headers;
using System.Threading;

namespace MF
{
    class MainClass
    {
        public static HttpClient client = new HttpClient();
        static string url = "http://www.website.com";

        public static void getSession() {
            StringContent queryString = new StringContent("{json:here}");

            // Send a request asynchronously continue when complete
            var result = client.PostAsync(new Uri(url), queryString).Result;

            // Check for success or throw exception
            string resultContent = result.Content.ReadAsStringAsync().Result;

            Console.WriteLine(resultContent);
        }
        public static void Run() 
        {
            getSession ();
          // Not yet implemented yet..
          //doMore ();
        }

        public static void Main (string[] args)
        {
            Console.WriteLine ("Welcome!");

            // Create the threads
            ThreadStart threadref1 = new ThreadStart(Run);
            ThreadStart threadref2 = new ThreadStart(Run);
            Console.WriteLine("In the main: Creating the threads...");
            Thread Thread1 = new Thread(threadref1);
            Thread Thread2 = new Thread(threadref1);

            // Start the thread
            Thread1.Start();
            Thread2.Start();
        }
    }
}

Also, I am not sure if it matters but I am running this on my MacBook Pro and plan to run it on my BeagleBoard.

C O
  • 326
  • 1
  • 4
  • 11
  • 2
    `getSession` definitely should be `async`. Calling async methods, such as `PostAsync`, and waiting for result, is not a way to go; do not share `HttpClient` between threads; do not create separate threads by hand, just call async method N times to create N tasks. – Dennis Jan 13 '16 at 07:34
  • 2
    Documentation states that it isn't thread safe. You can create new instances of `HttpClient` for each request. I hope your application can scale well if you prefer asynchrony over multithreading. Instead of using Result property, await the Task. – Sriram Sakthivel Jan 13 '16 at 07:34
  • Possible duplicate of [Is HttpClient safe to use concurrently?](http://stackoverflow.com/questions/11178220/is-httpclient-safe-to-use-concurrently) – CMircea Jan 13 '16 at 07:40
  • Did it really? I read another documentation stating that it was Thread safe, I must have read a discreditable article. – C O Jan 13 '16 at 07:51
  • @Dennis Ah okay, that's the answer I was hoping to hear. In Java I was told to share the HttpClient, and that it's ment for sharing. Even on the apache site it's shared. My program will be sending a bunch of requests until a 200 code is received. I was hoping to run 5 tasks/threads, instead of opening the program 5 times. Would async still be the proper approach? I read an article that multithreading HttpClient was much faster. Again, thanks for taking the time to respond. I really appreciate the constructive criticism. – C O Jan 13 '16 at 07:57
  • 1
    I updated the answer with small todo list, let me know if you want pseudo-code in the answer as well – jpou Jan 13 '16 at 09:13
  • So, when the server is under heavy load, you want to throw more processing its way? Surely a single request at a time and some form of back-off strategy would be kinder to the server? (I.e. if everyone is interacting with the server in the same way, what you're actually doing is driving the load even higher and *reducing* the chances of the server being able to respond) – Damien_The_Unbeliever Jan 13 '16 at 09:25
  • @damien_the_unbeliever I have no control of the server, it's something that can't be helped. – C O Jan 13 '16 at 19:27
  • @jpou Thanks jpou, a psuedo code would be perfect just to get me going the right direction. It's been awhile since I touched C#. – C O Jan 13 '16 at 19:27

4 Answers4

11

Here is how I would do it if I were you. I would leverage async as much as possible as it's much more efficient than using Threads (you most likely won't have to context-switch all the time which is expensive).

class MainClass
{
    public static HttpClient client = new HttpClient();
    static string url = "http://www.website.com";

    public static async Task getSessionAsync()
    {
        StringContent queryString = new StringContent("{json:here}");

        // Send a request asynchronously continue when complete
        using (HttpResponseMessage result = await client.PostAsync(url, queryString))
        {
            // Check for success or throw exception
            string resultContent = await result.Content.ReadAsStringAsync();
            Console.WriteLine(resultContent);
        }
    }

    public static async Task RunAsync()
    {
        await getSessionAsync();
        // Not yet implemented yet..
        //doMore ();
    }

    public static void Main(string[] args)
    {
        Console.WriteLine("Welcome!");

        const int parallelRequests = 5;
        // Send the request X times in parallel
        Task.WhenAll(Enumerable.Range(1, parallelRequests).Select(i => RunAsync())).GetAwaiter().GetResult();

        // It would be better to do Task.WhenAny() in a while loop until one of the task succeeds
        // We could add cancellation of other tasks once we get a successful response
    }
}

Note that I do agree with @Damien_The_Unbeliever: if the server has issues under heavy load, you shouldn't be adding unnecessary load (doing X times the same request) and contribute to the server's issues. Ideally you'd fix the server code but I can understand that it's not yours.

bastuc
  • 138
  • 8
  • This is the perfect response, it's exactly what I was hoping for! I do realize it's silly to add unnecessary load to the server by adding more requests but it's something that just cannot be fixed via server side. It's beyond my control. – C O Jan 13 '16 at 18:50
  • The code above worked a lot faster for me already, and I changed the last line to Task.WhenAll(RunAsync(), RunAsync()).GetAwaiter().GetResult(); considering that I also want to execute doMore(); after getSessionAsync(); – C O Jan 14 '16 at 00:18
5

Messing with the headers is not thread safe.

For example, swapping out an OAuth access token with a new one is not thread safe. Facing this right now with a Xamarin mobile app. I have a crash report where one thread was modifying headers while another thread was attempting a request.

chad
  • 2,542
  • 1
  • 21
  • 6
3

The following methods are thread-safe:

CancelPendingRequests
DeleteAsync
GetAsync
GetByteArrayAsync
GetStreamAsync
GetStringAsync
PostAsync
PutAsync
SendAsync

More details:

Community
  • 1
  • 1
CMircea
  • 3,543
  • 2
  • 36
  • 58
  • I believe I am using PostAsync & GetAsync in my code. – C O Jan 13 '16 at 07:54
  • @CO then it'll be fine. – CMircea Jan 14 '16 at 11:56
  • Well the msdn posted above said it wasn't worth the risk. What are the risks? – C O Jan 16 '16 at 01:05
  • @CO that is standard text that appears on MSDN. Not all public methods or properties of HttpClient are thread safe (you shouldn't change DefaultRequestHeads from multiple threads), but the ones actually issuring requests are safe to use concurrently. Henrik F Nielsen, one of the designers of HttpClient, wrote in an article that it was designed to be used concurrently: http://blogs.msdn.com/b/henrikn/archive/2012/08/07/httpclient-httpclienthandler-and-httpwebrequesthandler.aspx – CMircea Jan 16 '16 at 08:55
2

Reading the docummentation of HttpClient:

Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

Don't take risks. Have a separate HTTP client per thread.

It seems that you are blocking the thread waiting for the reply and you are doing it from a thread that does not do anything extra. Then why bother with async/await? You can use plain simple blocking calls.

Also - your program now finishes immediately after starting the threads. You might want to wait for the threads to finish before returning from main. You can do that by this code at the end of your program:

Thread1.Join();
Thread2.Join();

Update based on comments:

  1. Decide how many parallel requests you want to make - this will be your number of threads.
  2. Make main thread wait for signal using ManualResetEvent.
  3. In each thread keep submitting your requests. As soon as you get answer that you are waiting for - signal the ManualResetEvent and allow your main function to return.
jpou
  • 1,935
  • 2
  • 21
  • 30
  • Hmm, I do not wish to block any calls but instead run the same POST requests simutaneously until a 200 code is received. What issues may arise if the program closes after starting the thread? I don't need the program to continue to run after the getSession(). – C O Jan 13 '16 at 07:54
  • As it stands now, your program will finish as soon Thread2.Start() is completed. When your program finishes running - OS will take care to destroy all threads started and you will not get any outcome. – jpou Jan 13 '16 at 09:07