After Thread implementation code not working as assumed

Question

5.00/5 (1 vote)

See more:

I got a loop which inside i've placed Execute method which is responsible for sending web request towards some server. Its working fine but its taking too long foreach request response. I decided to implement threads instead. Unfortunetly after implementation code is not working as expected. Let me explain you firstly what code is doing. My execute method taking couple variables: first one takes xmlsoap file request path, second file path which result will be stored for each element, third is for inventory element name, fourth Timeout file and last one server towards which i am sending those requests. The way of working is Execute method is taking first parameter and sending out request then if the result is comming up saves it to second parameter file (DONE file with the element name) and going next. It could be possible that for specific request element will be timeout that means result is not comming out then the name of element is saving into Timeout file (saving is done in catch clause) and DONE file is not creating. All works perfectly if i am doing it synchronusly. But after implemented solution Threads results comming up very fast (good) new results cames around (DONE file for each possitive retreived result element) were appearing in my folder . BUT... From investigation i've found out that too many elements names are going to TimeOut file and even some of them are duplicated there. The way is if element X went to TimeOut file then DONE file containing result is not created because there was timeout which is equal to no result. If its really true there was timeout for some element the name of element should be putted to TimeOutFile and only once. Second thing i saw some elements are inside TimeOut file and Done file response is created as well (strange). Could you please help me out what can be wrong in this situation? Hope explained you enough good.

This is the method i am using:

C#

public static void Execute(string xmlFileRequest, string xmlFileResult, string InventoryElement, string TimeOutFile, Server server)
       {
            HttpWebRequest request = null;
            try
            {

                System.Net.ServicePointManager.DefaultConnectionLimit = 100;
                request = (HttpWebRequest)WebRequest.Create(server.ServerIP);
                //request.ServicePoint.ConnectionLimit = 1000;
                request.Headers.Add(@"SOAP:Action");
                request.ContentType = "text/xml;charset=\"utf-8\"";
                request.Accept = "text/xml";
                request.Method = "POST";
                request.KeepAlive = false;
                var soapEnvelopeXml = new XmlDocument();
                soapEnvelopeXml.Load(xmlFileRequest);

                try
                {
                    using (Stream stream = request.GetRequestStream())
                    {
                        soapEnvelopeXml.Save(stream);
                    }
                }
                catch (Exception ex)
                {
                }
                try
                {
                    using (WebResponse response = request.GetResponse())
                    {
                        using (var rd = new StreamReader(response.GetResponseStream()))
                        {
                            string soapResult = rd.ReadToEnd();

                            var xdoc = new XmlDocument();
                            xdoc.LoadXml(soapResult);
                            xdoc.Save(xmlFileResult);
                        }
                    }
                }
                catch (WebException e)
                {
                    Monitor.Enter(locker);
                    {
                        using (StreamWriter sw = File.AppendText(TimeOutFile))
                        {
                            sw.WriteLine(InventoryElement);
                        }
                    }
                    Monitor.Exit(locker);
                }
            }
            catch
            {
            }
            finally
            {
                if (request != null)
                {
                    request = null;
                }
            }
       }

and the Thread loop over that method:

C#

List<Thread> myThreads = new List<Thread>();
      foreach (MakeRequest item in makeRequestList)
      {
          Thread thread = new Thread(() => Execute(item.file_READYTOTEST, item.file_DONEFILE, item.Inventory_name, IO.TimeOutFP, item.server));
          thread.Start();
          myThreads.Add(thread);

      }

      foreach (var thread in myThreads)
          thread.Join();

If you need more information or something is not clear please write me question. Thank you.

Posted 26-Aug-14 12:06pm

carlito_brigante

Add a Solution

Comments

Sergey Alexandrovich Kryukov 26-Aug-14 18:16pm

First of all, I up-voted this question. After weeks of questions which make no sense, provide no information at all or discuss the wildest abuse, this is the first perfectly sensible and pretty well explained, adequate post with adequate level of detail. Besides, I appreciate that you suggested a reasonable way of passing multiple parameters to a thread method, in contrast to idiotic ParametrizedThreadStart (I mean it: idiotic: totally redundant, pointless and relatively unsafe). Further consideration shows problem but... the correctly explained problems.
—SA

carlito_brigante 27-Aug-14 5:31am

Hi Sergey, first all thanks for such cain of post :). I've tested once again without Thread implementation so what i did is between foreach statment i stayed only Execute method without Threading. The result was as it should be. By the way i received information from guy which is taking care server i am sending requests and he told me that asynchronous is allowed towards the server but up to 5 concurrent requests. Is it possible somehow to modify my code to send only 5 requests then wait on results and then send next 5 and wait on results and so on? Would you be abble to help me out how i can do that on my example code?

P.S in case of XMLDocumet i think i duplicated it inside Execute method could you also help me out how can i modyfy to use Load only one time i think i am doing it double time without reasons.

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Sergey Alexandrovich Kryukov · Answer 1 · 2014-08-26T12:51:00

It seems to me that major problem is not in your code, but on the server side. From your question, I probably can assume that if you do the same operations sequentially, your client works find. Please confirm it. If you did not explicitly tried it with your new code, please do it. The simplest way to try it is this: make sure your makeRequestList has only one element. Better yet, simply call Execute under the loop directly, in the calling thread. If it works slowly and gives your satisfactory results (slowly), the server side is almost certainly a problems.

Now, let's see what your threading can possibly give you. Imaging you execute several threads in parallel, each working with different remote host. In this case, the gain is apparent: the bottleneck is either in the network traffic somewhere in the middle of the network, or at the service. In all cases, your CPU usage is not a bottleneck: the client system rarely extensively use the network elsewhere, so your threads spend considerable time in wait state, waiting for more data in networks streams, thus not spending CPU resources in vain. Everything would be good.

What happens if you poll the same very service host? Well, it depends. If the service is a big Web factory with load balancing and other powerful things, your threading approach will work. But if the host is just a single host computer with the 2-4 cores, the Web service/site/application can become a bottleneck, so most of your threads would be wasted. What is your cases? what's in the request lists? If those are the requests to the same host, having all your threads might not be a solution. Such solution can make things even slower, because of the overhead of the threading itself. You might need a sequential set of request, or just a few active threads at a time, say, 2-6. You can try it out to figure out the close-to-optimum number (when your functionality is fixed, of course).

But your problem is more serious. Timeouts. If my assumption I formulated above in my first paragraph is correct, the server side is a problem. You overwhelm the host with too many request, the responses take too much time for such settings; and you get many timeouts. Not much you can do with this situation, unless you address the problem on the service. Even if you make your data downloads "more sequential", some other client may overwhelm the server part. This is not right.

Now, about some problems of your code. First, creating all threads in the second fragment of code shown is not good. You should better use the thread from thread pool, or reuse some fixed set of threads, which I personally prefer to do. A thread is sleeping at the blocking call (such as with the use of EventWaitHandle) and awaken when a task is ready to start. On comprehensive approach is using the System.Collections.Concurrent.BlockingCollectio<code>n<t></t>. See also my article where I explain all the important detail and provide interesting usage samples: Simple Blocking Queue for Thread Communication and Inter-thread Invocation[^].

I already appreciated your way of passing multiple parameters to a thread. However, I know even better way, with a thread wrapper. Please see my past answers:
How to pass ref parameter to the thread[^],
Change parameters of thread (producer) after it is started[^],
MultiThreading in C#[^];
see also: Running exactly one job/thread/process in webservice that will never get terminated (asp.net)[^],
Making Code Thread Safe[^].

I would question the general code design, where you get data from files, put to other files, many files… However, I don't know your problem well enough to be sure, maybe it makes sense.
The big problem would be the blocking of the exception propagation using … catch { }. There are few cases when it makes sense, mostly if you have to fix some defects in 3rd-party code which is not accessible for patching.
And, finally, I hope you understand that hard-coding any immediate constants like your 100, @"SOAP:Action", and so on, is not good; it's much better to use explicitly declared constants (and even resources, data files, etc., in other cases).

—SA