 |
 | Home | Contact Us | FAQ | Search & Site Map | Link to Us |
 | Sign In | Join | Other 45 Sites in Network |
.NET Forum / Languages / C# / August 2007multithreaded tcp/ip monitoring application
hi, I am new to c# development. I need to develop a windows application that connects remotely to another application using tcp/ip and gathers statistics like, application running or not, application uptime and also some more data that the other application sends to this monitoring application. Also at the same time i need to update the main dialog form with all this data. In the background, It needs to monitor the remote application continuously and collect all the data. Can someone please suggest how to build such an application. Do i need to have a multithreaded application. If yes, please suggest some useful links. I need to use c#. Thank you > I am new to c# development. I need to develop a windows application > that [quoted text clipped - 13 lines] > links. I > need to use c#. You do not need an application that is explicitly multithreaded. However, having _some_ sort of threading is beneficial and IMHO necessary. See the Socket class for particulars, and especially the BeginSend, EndSend, BeginReceive, and EndReceive methods which offer asynchronous i/o mechanisms that will allow you handle the Socket i/o without interfering with the handling of the user interface on the main thread. Because those methods will cause your code to run on threads other than the main thread owning the UI, you will also want to look at the Control class (a base class for the Form class), and its methods Invoke and BeginInvoke. Those are required for handling UI updates from threads that are not the owner of the Control. Now, all that said, if you are new to .NET, you have some learning to do before you can effectively use the above. A general explanation of all that is outside the scope of this newsgroup, but of course feel free to post specific questions as they come up. If you are simply new to C#, but are familiar with the .NET Framework, then you may do fine simply reviewing the documentation for the above. Pete Thanks a lot for your suggestion. I have read concepts related to sockets, Begin receive and End receive. I have two pieces of code and not sure which is the right way to develop the application. I can give u a brief idea and may be u could guide me which way to adopt and if my coding mechanism is right. In my application I intend to connect to a remote pc and I do that in my btnclick event as follows-- private void btnConnect_Click(object sender, EventArgs e) { //some code validating the ipaddress entered in txtbox lblConnStatus.Text = "Trying to establish connection...."; // reset events m_EventStopThread.Reset(); m_EventThreadStopped.Reset(); // create worker thread instance that connects with the Network Recorder periodically and checks its status m_WorkerThread = new Thread(new ThreadStart(this.WorkerThreadFunction)); m_WorkerThread.Start(); btnConnect.Enabled = false; DateTime CurrTime = DateTime.Now; lblStatusSince.Text = CurrTime.ToShortDateString()+ " " + CurrTime.ToShortTimeString(); } The worker thread calls a Start Client function which continuously tries to connect to the server in a while loop. Like as in the following code: public void WorkerThreadFunction() { IPAddress ip = IPAddress.Parse(tbxIPAddress.Text); while (true) { // some stmts if (client != null) { client.Close(); } client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); StartClient(ip, client, this); Thread.Sleep(1500); connected = true; } StartClient function looks like this private static void StartClient(IPAddress ip, Socket client, MainForm m_form) { // Connect to a remote PC. try { IPEndPoint remoteEP = new IPEndPoint(ip, 8001); // Connect to the remote endpoint. client.BeginConnect(remoteEP, new AsyncCallback(ConnectCallback), client); connectDone.WaitOne(1000, false); client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.SendTimeout, 1000); Send(client, "This is a test<EOF>"); sendDone.WaitOne(1000, false); client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReceiveTimeout, 1000); Receive(client); status = true; // Remote Application running. Set the Application icon to green m_form.Invoke(m_form.m_DelegateSetAppIcon); } catch (Exception e) { // Remote application not running. Set the Application Icon to Red status = false; m_form.Invoke(m_form.m_DelegateSetAppIcon); //MessageBox.Show(e.Message); } } In the above code.... The client tried to continuously connect to the remote server. The previous connect calls are nullified by assigning null to the client each time the external loop is called. This is one way of making sure that the remote application is alive. Another way I thought was to connec to the application just once and call the Begin Receive function... and and call beginreceive repeatedly. If in between the server shuts down, beginreceive will have a socketexception which needs to appropriately handled by just updating the UI to show that the remote application is down and then retry connecting repeatedly until the connection is established and then again start receving data. Can you please tell me which way is better. The first code seems to be working. But I find the second one less complicated. But I am having trouble updating the UI from OnReceive function for which I will post my code to take your suggestions. Sorry for the very long post. Please reply asap. Thank you The second method that I adopted looks like this: private void btnConnect_Click(object sender, EventArgs e) { IPAddr = tbxIPAddress.Text; //Validate IP Address if (!IsValidIPAddress(IPAddr)) { MessageBox.Show("Invalid IP Address", "Input Error", MessageBoxButtons.OK, MessageBoxIcon.Error); return; } //Storing the IP Address in file for next time // FileStream file = new FileStream("C:\\IPAddress.txt", FileMode.Create, FileAccess.Write); // StreamWriter sw = new StreamWriter(file); // sw.Write(IPAddr); // sw.Close(); //Connecting to the Network Recorder this.lblConnStatus.Text = "Trying to Establish Connection..."; Application.DoEvents(); System.Threading.Thread.Sleep(3000); port = 1008; try { server = new TcpClient(IPAddr, port); state = new StateObject(); state.workSocket = server.Client; server.Client.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback((new RecorderMonitor()).OnReceive), state); status = true; this.Invoke(this.m_DelegateSetAppParam); } catch (SocketException) { status = false; RetryConnect(this); //return; } } RetryConnect functions looks like this public void RetryConnect(RecorderMonitor m_Form) { //this.lblConnStatus.Text = "Retrying to Establish Connection..."; Application.DoEvents(); System.Threading.Thread.Sleep(5000); try { server = new TcpClient(IPAddr, port); state = new StateObject(); state.workSocket = server.Client; server.Client.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback((new RecorderMonitor()).OnReceive), state); } catch (SocketException) { status = false; if (this.lblConnStatus.InvokeRequired) { RetryConnectCallBack d = new RetryConnectCallBack(RetryConnect); this.Invoke(d, new object[] { m_Form }); } else { m_Form.lblConnStatus.Text = "Network Recorder is down"; // m_Form.btnConnect.BackColor = System.Drawing.Color.Red; Application.DoEvents(); } return; } In the btnconnect event... I just tryt o connect to the remote application only once and repeatedly try connecting to it unless the connection is established. I am not having any trouble in doing that. The trouble comes when I am connected to the server and start receiving the Data using the Begin Receive which calls the following function: public void OnReceive(IAsyncResult ar) { String content = String.Empty; // Retrieve the state object and the handler socket // from the asynchronous state object. StateObject state = (StateObject)ar.AsyncState; Socket handler = state.workSocket; int bytesRead; if (handler.Connected) { // Read data from the client socket. try { bytesRead = handler.EndReceive(ar); if (bytesRead > 0) { // There might be more data, so store the data received so far. state.sb.Remove(0, state.sb.Length); state.sb.Append(Encoding.ASCII.GetString( state.buffer, 0, bytesRead)); // Display Text in Rich Text Box // content = state.sb.ToString(); // Console.WriteLine(content); handler.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(OnReceive), state); } } In the above code, it repeatedly keeps calling BeginReceive to ensure it is continuously r eceiving data from server... But now I need to handle a situation where if the server goes down.. the catch block tries to retry connecting as it was doing in the previous case. Here whenever I call RetryConnect function, it says invalid operationexception after executing that function. Basicallu its not able to update the UI ... may be bcause in this case RetryConnect belongs to a different thread. How should update the UI just like I was doing before from btnConnect event.. in this case. I even tried to declare RetryConnect as a Delegate but it doesnt work. Please suggest. Sorry forgot to post the Catch block for the second case catch (SocketException socketException) { //WSAECONNRESET, the other side closed impolitely if (socketException.ErrorCode == 10054 || ((socketException.ErrorCode != 10004) && (socketException.ErrorCode != 10053))) { //handler.Close(); status = false; // RetryConnect(this); this.Invoke(this.m_DelegateSetAppParam); // this.lblConnStatus.Text = "Network Recorder is down"; // Application.DoEvents(); } } If I try to use Invoke , it says you are using the windows handle even before creating it. The OnReceive function doesnt have the reference to the applications window's handle. How should I get that? No offense intended, but there are so many things wrong with the code you posted, it's hard to know where to start. So, I guess I will just start at the top. Please note that you may find this will be an iterative process. Many of the problems are not directly related to how the basic functionality you want should be architected, though there may be hints there. I'll try to put some sort of useful pseudocode at the end as a way of suggesting the correct architecture. So, on with the comments... > The second method that I adopted looks like this: > > private void btnConnect_Click(object sender, EventArgs e) > { > > IPAddr = tbxIPAddress.Text; This is not great: > //Validate IP Address > if (!IsValidIPAddress(IPAddr)) [quoted text clipped - 3 lines] > return; > } There's no need to validate the IP address. When you pass it to the TcpClient, it will validate it for you. If the string cannot be resolved, you'll get an exception. Handle the exception then, converting to plain English if appropriate (you'll get socket error that relates to the invalid IP address. If your code concerns itself with the specifics of how an IP address looks, you lose out on functionality like using IPv6 and named hosts (things that TcpClient should handle automatically for you if you let it). This part is awful: > //Storing the IP Address in file for next time > // FileStream file = new FileStream("C:\\IPAddress.txt", > FileMode.Create, FileAccess.Write); > // StreamWriter sw = new StreamWriter(file); > // sw.Write(IPAddr); > // sw.Close(); You should not be saving a file to the root directory of the C: drive, and you should not really be using a file like this anyway. Just create a "User" scope setting in your project to store the IP address. You can access it using Properties.Settings.Default.IPAddress (assuming you call it "IPAddress"). After setting it, you need to call Properties.Settings.Default.Save(), to make sure it's stored in the user.config file. You can retrieve it at any time; it will persist in the user.config file after you call Save() and will be available when you run the application next time (and any time after that). > //Connecting to the Network Recorder > > this.lblConnStatus.Text = "Trying to Establish > Connection..."; This part is awful: > Application.DoEvents(); > System.Threading.Thread.Sleep(3000); There is no need to ever call DoEvents() in properly architected code. There is certainly no need here, as you have not done anything at this point that would block the code for any significant period of time before calling DoEvents(). It's unlikely the call to DoEvents() will do anything significant. As for the call to Sleep()...what are you thinking here? You are just arbitrarily stopping program execution for 3 seconds. What's the point of that? What are you waiting for, and why does it seem reasonable to make the user wait 3 seconds? > port = 1008; > try > { This part is not great: > server = new TcpClient(IPAddr, port); You want to use TcpClient asynchronously. So use BeginConnect() as well as BeginReceive(). Not only does it avoid the UI updating issues, it makes it a lot easier to address the retry-on-failure design you're trying to accomplish here. > state = new StateObject(); > state.workSocket = server.Client; This part is awful: > server.Client.BeginReceive(state.buffer, 0, > StateObject.BufferSize, 0, new AsyncCallback((new > RecorderMonitor()).OnReceive), state); You are creating a new instance of your form class here. Bad! You never show it, which is why later on you get the exception you mentioned in a follow-up to this post (using the window handle before creating it). But beyond that, I see nothing in your posts that suggest you want a new instance of the form. You want to use the existing form, the one that the user has been interacting with already. So use that one: "new AsyncCallback(OnReceive)". This part is not great: > status = true; > this.Invoke(this.m_DelegateSetAppParam); > } The "status" flag is almost certainly superfluous. There are likely better ways to accomplish whatever it is you think you're doing there. But more significantly, the call to Invoke() is completely superfluous. You are handling the button click event. You are guaranteed to be in the form's owning thread. There is absolutely no need to use Invoke() in this context. Just call the delegate directly: "this.m_DelegateSetAppParam();". That, of course, assumes that there's a real point to that delegate. I'll take as granted that there is. This part is not great: > catch (SocketException) > { > status = false; > RetryConnect(this); > //return; > } In particular, you should at least check the socker error in the exception. Not all exceptions should lead you to think that retrying the connection attempt would actually work. There are certain exceptions for which you can just give up now. > } If you use BeginConnect(), then there is no need for a "retry" function. The callback provided to BeginConnect() can simply reissue the BeginConnect() when appropriate, specifying itself as the callback. Note that this retry function has the same things wrong with as the button click handler: calling DoEvents(), arbitrarily sleeping for 5 seconds, creating a new RecorderMonitor() instance, etc. In addition... > RetryConnect functions looks like this > [quoted text clipped - 18 lines] > > status = false; This is not great: > if (this.lblConnStatus.InvokeRequired) > { [quoted text clipped - 10 lines] > Application.DoEvents(); > } Some things wrong with the above block: * If you do the Begin/EndXXX stuff correctly, there is no need to check InvokeRequired. In your callbacks, you will be guaranteed that you need to call Invoke() and in the UI code you will be guaranteed that you don't need to. That's one advantage (of many) with respect to using the async pattern correctly. * The code has this very peculiar trait that it only treats an exception as fatal ("Network Recorder is down") once it's been run on the main thread. So, if you call it while the initial connect is attempted, a failure here is fatal. But if you call it from your asychronous BeginReceive() callback, a failure here results in a retry. That retry gets put on the main thread, and so a subsequent failure there _is_ fatal. While I don't see any obvious harm in this design, it's overly complicated. Overly complicated often leads to hard-to-find bugs. It's just not a good idea. * You call DoEvents(). For the same reason this is bad elsewhere, it's bad here. > return; > } [quoted text clipped - 3 lines] > only once and repeatedly try connecting to it unless the connection is > established. I don't see a repeated attempt to connect. As near as I can tell, you make one retry attempt after the initial failure, and then give up. This may or may not be correct, depending on what your intent was. > I am not having any trouble in doing that. The trouble comes > when I am connected to the server and start receiving the Data using the > Begin Receive which calls the following function: As with the previous code, there are problems. More comments... > public void OnReceive(IAsyncResult ar) > { [quoted text clipped - 5 lines] > Socket handler = state.workSocket; > int bytesRead; This is not great: > if (handler.Connected) Don't bother checking for connected. If the socket has become disconnected, you'll find out when you call EndReceive() and get an exception. This if() statement is superfluous, and complicates your flow control in the function (in particular, you have no retry logic if the socket is not connected). > { > [quoted text clipped - 9 lines] > state.sb.Append(Encoding.ASCII.GetString( > state.buffer, 0, bytesRead)); This is awful: > // Display Text in Rich Text Box > // content = state.sb.ToString(); You are guaranteed at this point to be on a different thread from the main UI thread. You need Invoke() or BeginInvoke() if you are going to set something in the main form directly or otherwise run some code on the main UI thread (which is not a bad way to synchronize access to shared data structures in a form, even if it doesn't require updating something in the displayed form itself). Alternatively, you need to use some multi-thread-synchronized data object to pass data from your i/o worker thread to whatever thread in your application will process it. > // Console.WriteLine(content); > [quoted text clipped - 14 lines] > in > this case RetryConnect belongs to a different thread. The exception isn't because of a thread issue. It's because you created a new instance of your form and never showed it, when initializing the BeginReceive() callback. > How should update the > UI just like I was doing before from btnConnect event.. in this case. I > even > tried to declare RetryConnect as a Delegate but it doesnt work. Here's the basics: Main thread: -- button click: create TcpClient, call TcpClient.BeginConnect(string, Int32) Worker callbacks (which run on threads that are automatically managed for you): -- connect callback: call EndConnect(), if successful then call BeginReceive(), if not then use Invoke() to update form's status text and call BeginConnect(string, Int32) again if appropriate (don't do this if you want the retrying to stop) -- receive callback: call EndReceive(), if successful then use Invoke() to update UI with received data for status and call BeginReceive() again, if failed then use Invoke() to update UI with status and call BeginConnect(string, Int32) to try again (and as above, only if you want to retry) If the callback methods are instance methods in your form class (as they appear to be in your posted code), then you always have access to your form instance members from the callback methods. Do _not_ create a new instance of your form when providing the callback methods. Hope that helps. Pete Thank you very much for all your suggestions. Now certain things are clear to me. I would like to outline the design I can think of now after reading your entire post. This is what I understand: I basically don't need to use any threads. In my btnConnect event, I will call BeginConnect and in the ConnectCall back, call End Connect. In my btnconnect itself, I can add the following statement : connectDone.WaitOne(1000, false); This will ensure that I wait for the connection to be successful before I start receiving data using BeginReceive in the next stmt within the btnConnect function. Similarly add a Receive Call back where I call end Receive and upon any exceptions, handle UI updates by using invoke(from withing the Receive Call back function) on the existing form instance. Within Receive after one receive operation is over, Call Begin Receive and have the receive operation running forever. So I wont be using any sort of while loops or threads and All the main function calls are done in the btnconnect event. Please let me know if I understood correctly and if the waitOne statement that I am using is correct. Meanwhile I will try to implement the same. Thank you very much. > [...] > This is what I understand: You almost have it. > I basically don't need to use any threads. Not explicitly, no. That is correct. > In my btnConnect event, I will > call BeginConnect and in the ConnectCall back, call End Connect. Yes, this is also correct. > In my > btnconnect itself, I can add the following statement : [quoted text clipped - 3 lines] > start receiving data using BeginReceive in the next stmt within the > btnConnect function. No, this is not correct. As I indicated in my previous reply, the only TcpClient thing you need to do in the main thread is create the instance and call BeginConnect(). Everything else happens on worker threads, in the callback methods. Here, as I also indicated, the correct place to make your first call to BeginReceive() is in the callback for the BeginConnect() method. That is, the method in which you call EndConnect(). After calling EndConnect(), assuming no error happens, then you call BeginReceive() there in the callback method. The "connectDone" event handle is simply not needed. Dealing with the processing that needs to happen after the connection completes successfully is best done in the connection callback itself. Once the main thread in your button click handler has called BeginConnect(), it has "set the wheels in motion" so to speak and doesn't have to do anything else (except for some UI update if you like, of course). > Similarly add a Receive Call back where I call end Receive and upon any > exceptions, handle UI updates by using invoke(from withing the Receive > Call > back function) on the existing form instance. Yes, this is correct. > Within Receive after one receive operation is over, Call Begin Receive > and > have the receive operation running forever. Yes, this is also correct. Your calls to BeginReceive() basically continue indefinitely until an error occurs (for example, the connection is forcibly reset for some reason) or you detect graceful closure of the connection (0 bytes are received). > So I wont be using any sort of while loops or threads and All the main > function calls are done in the btnconnect event. Almost. No loops or explicit threads are needed. The _only_ thing the "btnconnect event" handler needs to do with the TcpClient is call BeginConnect(). Everything else is done in the callbacks. > Please let me know if I understood correctly and if the waitOne statement > that I am using is correct. It is not. See above. :) > Meanwhile I will try to implement the same. Good luck! Pete Also I was using Sleep for 3 seconds and Application.DoEvents() because I ws trying to update a labels text and it was not updating it. After I added Doevents, it got updated. And I added sleep so that that text " Trying to establish Connection" remain for sometime.. to show the user that the connection to the remote application was being made. Just wanted it to be a little fancy. Any better suggestion to implement this? > Also I was using Sleep for 3 seconds and Application.DoEvents() because > I ws > trying to update a labels text and it was not updating it. After I added > Doevents, it got updated. Sleeping for three seconds did not do anything to change the updating, other than to make the user wait arbitrarily for three seconds before the connection could proceed. As for calling DoEvents(), yes...that allows the UI to update itself. But a) it is "better" in that situation to simply call Control.Update() on the control you've changed, and b) it is even better to not have the main UI thread block in the first place (for example, doing a synchronous call to TcpClient.Connect()). If you use the mechanisms I am suggesting here, your change to the text label will be displayed nearly as soon as it's been made in your code, and will not be delayed by the time it takes to successfully complete the TCP connection. Note that in reality, calling Control.Update() or Control.Refresh() is usually almost as bad as calling DoEvents(). Any of those methods is frequently an indication of a design problem, since they are almost never actually required. > And I added sleep so that that text " Trying to establish Connection" > remain [quoted text clipped - 3 lines] > suggestion to > implement this? Implement the TcpClient stuff using the Begin/EndXXX mechanism, and everything else will work fine. Also, I will point out that I see to usefulness in putting a Sleep() in your code just so the user can see some particular text you've set. If the text would otherwise be immediately changed to reflect a successful connection, what point is there in wasting three seconds telling the user that you are _trying_ to establish the connection. For one thing, during the three seconds you're sitting there, you're not actually trying to establish the connection. So you're lying to the user. For another, if the connection completes so quickly that the user never sees the phrase "Trying to establish connection", so what? Presumably the next phrase will be "Connection established" or something like that, which implies to the user that you did in fact try to establish the connection. Don't put arbitrary delays into your code. They just waste the user's time. Pete Hi, Thanks a lot foryour help. I can get the application working as of now. But I still feel, it might not work in certain cases because I have made superflous assumptions and I am not sure how to handle certain error checks/cases. For example, I have written the client code in C# following your suggestions. My server side code is in VC++. Since I am monitoring the status of an application, I added the server side code to the application that is being monitored. That application sends data about its current status( its doing some stuff and it sends information about that to the client). I had a monitor thread running on the server side that continously sends updated application statistics to the monitoring application. Now the problem is... the sending of the data is so fast that the receiving side is not able to keep up with it. From the server side I am sending data in a comma separated fashion. And each data packet I send has constant length 61 bytes. If If do not add any delay in send, the receive at the client side receives 2-3 data packets and then is not able to parse the received data properly. So I added a delay of 5 sec on server side. So that the client has enough time to process each packet it receives. But I am sure this might not work always. I need some suggestions on how to synchronize the server and client. Should I send some acknowledgement back from the client everytime I receive a packet. I am new to c# as well as network programming. And I need someone to help me with the design. Once I build my first app, I am sure I will do good in subsequent apps. Pete, please give me some more pointers . I have seen some chat applications in C#.. but they do not have everything I need. Thanks a lot! Also I am unable to gracefully shut exit my application. When I close the application using the windows close button, there is no problem. But when I have my own exit button defined with the following code: private void btnExit_Click(object sender, EventArgs e) { if (client != null) { client.Close(); client = null; } Dispose(); Environment.Exit(0); } in the ReceiveCallBack method it throws an exception saying that I am accessing a disposed obect. How can I notify ReceiveCallback to stop receiving once the exit button is clicked. It is running as an independent thread and does not know when the client socket is closed and still tries to receive on that socket and throws an exception. How should I gracefully shut down my application. Thanks > [...] > private void btnExit_Click(object sender, EventArgs e) [quoted text clipped - 11 lines] > accessing a disposed obect. How can I notify ReceiveCallback to stop > receiving once the exit button is clicked. Two things wrong: * To perform a graceful close on the connection, you need to use Shutdown(). * To close the application, you should call Close() and not Dispose(). IMHO, calling Environment.Exit() is superfluous. If you have just the one form, closing the form should accomplish the same thing. As far as the exception in your ReceiveCallback goes, you can deal with this in at least two different ways: * Don't close the form in the button click handler. Instead, set a "volatile" flag indicating that the application should be shutdown, and then in your receive callback do the Close() on the form when you get the socket closure indication. * Don't try to access the form in the receive callback. Set a "volatile" flag indicating that you _have_ closed the form, or just check the to see if it's still open, and only try to access it if it is. Pete TCP doesn't do any sort of blocking on it's packets. Since you know your line length, you need to add each TCP packet to a master buffer as it arrives and then take your data chunks off that buffer Here's pseudo code - int count = recv(socket, out buffer) if count == 0 then the socket is gone. msg += buffer.ToString() while msg.Length > 61 string chunk = msg.Substr(0, 60) msg = msg.Substr(61, msg.Length-61) process(chunk) loop Although this error occurs mainly on fast connections, I have seen it occur over dialup connections as well. Mike. > Hi, > [quoted text clipped - 40 lines] > > Thanks a lot! > [...] > Now the problem is... the sending of the data is so fast that the [quoted text clipped - 7 lines] > receives 2-3 data packets and then is not able to parse the received data > properly. As Michael's response indicates, you need to delimit your data yourself. I suspect that you are not dealing with a bandwidth problem, but rather a logic error in your own code. Just because the sender sends in 61-byte increments, that doesn't mean that's how the data is sent out the network adapter, nor does it mean that's how the data is received on the receiving side. That's why you receive "2-3 data packets"...when several 61-byte chunks are sent in quick succession they are automatically coalesced by TCP. When you receive these, you need to break them apart yourself. Pete Actually I did not observe carefully, I am not receiving 2-3 packets. Infact I am receiving bits and portions of packets. Sometimes 2 packets and little of the third packet. The rest is received in the next receive function call. I am not receiving in multiples of 61 bytes. Since I am sending packets of 61 bytes each, why am I receiving 73 bytes, 195 bytes and so on? Shouldnt they be multiple of 61?? How should I handle this situation? Here is my OnReceive function that I modified according to your suggested pseudocode. It works without any runtime exceptions/errors but is not doing what is required. private void ReceiveCallback(IAsyncResult ar) { // Retrieve the state object and the client socket // from the asynchronous state object. // Read data from the remote device. try { int bytesRead = client.EndReceive(ar); char[] chars = new char[bytesRead]; if (bytesRead > 0) { // There might be more data, so store the data received so far. //state.sb.Append(Encoding.ASCII.GetString(state.buffer, 0, bytesRead)); System.Text.Decoder d = System.Text.Encoding.UTF8.GetDecoder(); int charLen = d.GetChars(state.buffer, 0, bytesRead, chars, 0); System.String szData = new System.String(chars); while (szData.Length >=61) { MessageBox.Show(szData.Length.ToString()); MessageBox.Show(szData); string chunk = szData.Substring(0, 61); if (szData.Length > 61) szData = szData.Substring(61, szData.Length - 61); else szData = ""; if (chunk.Length>0) { int i, j; for (i = 0; chunk[i] != ','; ++i) ; m_errorflag = chunk.Substring(0, i); for (j = i + 1; chunk[j] != ','; ++j) ; m_errormesg = chunk.Substring(i + 1, j - i - 1); for (i = j + 1; chunk[i] != ','; ++i) ; m_statussince = chunk.Substring(j + 1, i - j - 1); for (j = i + 1; chunk[j] != ','; ++j) ; m_compressfile_count = chunk.Substring(i + 1, j - i - 1); m_uptime = chunk.Substring(j + 1, chunk.Length - j - 1); } status = true; this.Invoke(this.m_DelegateSetAppParam); } client.BeginReceive(state.buffer, 0, state.buffer.Length, 0, new AsyncCallback(ReceiveCallback), null); } /* else { // All the data has arrived; put it in response. if (state.sb.Length > 1) { response = state.sb.ToString(); MessageBox.Show(response); } // Signal that all bytes have been received. receiveDone.Set(); }*/ } catch (SocketException ex) { if (ex.ErrorCode == 10054 || ((ex.ErrorCode != 10004) && (ex.ErrorCode != 10053))) { status = false; this.Invoke(this.m_DelegateSetAppParam); client.Close(); client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); client.BeginConnect(remoteEP, new AsyncCallback(ConnectCallback), client); if (status == true) { state = new StateObject(); state.workSocket = client; // Begin receiving the data from the remote device. client.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ReceiveCallback), state); } } } } > > [...] > > Now the problem is... the sending of the data is so fast that the [quoted text clipped - 21 lines] > > Pete > [...] Since I am sending packets of 61 > bytes each, why am I receiving 73 bytes, 195 bytes and so on? Shouldnt > they > be multiple of 61?? No. There is no requirement that the coalesced data is in multiples of the data that was sent. Michael's code isn't the most efficient way of processing the data, but it does do the job and I would expect it to be just fine for your particular situation. You'll notice that his code makes no assumptions about the grouping of the incoming data. It simply takes whatever new data is received and adds it to the end of his buffer. It then checks the length of the buffer, and as long as there is at least 61 bytes, he pulls off the first 61 bytes and processes them. In this way, it doesn't matter how the bytes are grouped when they are received. The bottom line here is that TCP is a stream. It is not message-oriented, and if you expect the data to be grouped in messages you are required to do all of that processing yourself. The only thing that TCP guarantees is correct byte ordering. > How should I handle this situation? You should either use Michael's code, or something equivalent to it. Pete Hi, Thank you for all the help. I successfully created the basic version of my app. Now I need to enhance this version by allowing multiple socket connections to monitor multiple instances of the same application on different machines. I am guessing I will have multiple threads running in the back ground which in turn will have their own receive,connect threads associated with their corresponding socket. But I am not sure how to implement it. Can someone suggest me how to manage connecting multiple sockets and data being received on each of them? The winforms design is as follows: On the left I have the ability to add/remove multiple applications to which I wish to connect to. When I right click on a particular IP address in the tree view, and click connect, a new thread should start that continuously monitors the remote application on a machine. So, If I have 10 IP addresses in the tree view, I should be able to connect to 10 remote applications to monitor their status. Basically I need 10 different simultaneoius connections to 10 different remote applications. Please suggest how to implement. Thanks Thanks a lot! > > [...] Since I am sending packets of 61 > > bytes each, why am I receiving 73 bytes, 195 bytes and so on? Shouldnt [quoted text clipped - 25 lines] > > Pete > Hi, > [quoted text clipped - 9 lines] > Can someone suggest me how to manage connecting multiple sockets and data > being received on each of them? Assuming you are still using the async paradigm -- that is, BeginConnect, BeginReceive, etc. -- then there is basically no additional work to support multiple connections, at least from the perspective of handling the i/o. You simply call BeginConnect() on each new socket you want to connect, and the rest will happen automatically. When the connection has completed, you'll wind up in your connection callback, which already has a call to BeginReceive(), so each socket will get a call to BeginReceive() as it connects, which will start the receiving process. At this point, .NET will assign a thread to handle each i/o completion as it occurs, and your socket i/o will automatically be run on the assigned thread. If any of the activity as a result of receiving data will require accessing data structures shared with the other connections, then you will need to synchronize that access. But that becomes less of a Socket question and more of a general multi-threading synchronization question. You should look into the thread synchronization mechanisms documented in MSDN, and if you have questions after that, post them. You can start by looking at the C# "lock" statement, and the Monitor class, which are two simple things that can be used to synchronize access to data between threads (and which can be used together as well, though doing so isn't required). > The winforms design is as follows: On the left I have the ability to > add/remove multiple applications to which I wish to connect to. When I right > click on a particular IP address in the tree view, and click connect, a new > thread should start that continuously monitors the remote application on a > machine. How do you update the information to the user as a result of the monitoring? If you use BeginInvoke() to add data to a text box, for example, no additional synchronization will be required. The BeginInvoke() will serialize (that is, make it happen serially) all access to the text box, preventing any conflicts (you won't be able to guarantee the order of the data being appended, but as long as for a given socket you don't call BeginReceive() again until you've called BeginInvoke() to update the UI, then at least data on a given socket will be assured of being displayed in order in the UI). > So, If I have 10 IP addresses in the tree view, I should be able to connect > to 10 remote applications to monitor their status. Basically I need 10 > different simultaneoius connections to 10 different remote applications. > > Please suggest how to implement. You haven't provided much in the way of details regarding how your UI actually works, so I can't really be more specific than the above. Hopefully that's enough to get you started. Pete Thank you very much for the pointers to get started. Regarding synchronization of shared objects, currently I have a user defined class called NRMonitor and it has the following structure: class NRMonitor { public connstatus; } I have an array of objects for the above class, to store data for multiple connections. And I also have an ArrayList so that I can store these objects as Link List, because whenever the user removes the connection from the List, I remove the corresponding object in the list. So basically multiple threads with their own receive/connect functions would be updating this datastructure. But since they will be accessing different elements within the array to update their corresponding connstatus variable value .. so do I still need synchronization in this case? Or if you can suggest me a simpler mechanism of storing this data and updating it from the threads? Also about the UI, I was thinking of adesign where, on the left I would have a tree view of the IP addresses of the remote machines. On the right I have a tab control which will show the parameters corresponding to the selected IP address on the left. So unlike my current application where I am updating my UI from within the Receive thread, here I am just planning to update the Array objects pertaining to each connection and just display the selected connection statistics. But I am really not sure how complicated the code would be to implement this... If you could also suggest me something on this that would be great!! Thank you very much > > Hi, > > [quoted text clipped - 63 lines] > > Pete > Thank you very much for the pointers to get started. Regarding > synchronization of shared objects, currently I have a user defined class [quoted text clipped - 4 lines] > public connstatus; > } What type is "connstatus"? > I have an array of objects for the above class, to store data for multiple > connections. And I also have an ArrayList so that I can store these objects > as Link List, because whenever the user removes the connection from the List, > I remove the corresponding object in the list. Not that it likely matters, but do you store them as an ArrayList or a LinkedList? The two are not the same. > So basically multiple threads with their own receive/connect functions would > be updating this datastructure. But since they will be accessing different > elements within the array to update their corresponding connstatus variable > value .. so do I still need synchronization in this case? Well, yes and no. You don't need to synchronize access to the ArrayList itself per se, since you are only changing the data structures referred to by the ArrayList, not the ArrayList itself. However, presumably at some point some code takes the information stored in those data structures and relates that to the user information somehow. At that point, access to the data structures themselves does need to be synchronized. So, in that scenario yes...you need to synchronize the access to the data structures themselves. > Or if you can suggest me a simpler mechanism of storing this data and > updating it from the threads? IMHO, Control.BeginInvoke() is the simplest. Use it to execute a delegate that will take the received data and actually do something useful with it, such as storing the information into your "connstatus" value (whatever that is). Use the form that contains your UI as the instance on which to call BeginInvoke(). (Don't confuse Control.BeginInvoke() with Delegate.BeginInvoke()...the former is what you want, the latter will just create the same synchronization issues you're trying to solve). > Also about the UI, I was thinking of adesign where, on the left I would have > a tree view of the IP addresses of the remote machines. On the right I have a > tab control which will show the parameters corresponding to the selected IP > address on the left. I don't know why you are using a TreeView...it seems to me that a plain ListBox or ListView would be fine for displaying a simple list of IP addresses. That said, I don't see any reason you _can't_ use a TreeView either. As far as the tab control goes, that should be fine as well. Presumably, you'll have code to update the currently displayed tab based on changes to the currently selected IP address as those changes occur, as well as update the newly displayed tab based on the current state of the selected IP address as the user switches tabs. If you use BeginInvoke() to execute the delegate that does the updating of the "connstatus" data structure, then you can safely force an update to the currently displayed tab at the same time, as well as safely read the current "connstatus" data any time you switch tabs. > So unlike my current application where I am updating my UI from within the > Receive thread, here I am just planning to update the Array objects > pertaining to each connection and just display the selected connection > statistics. But I am really not sure how complicated the code would be to > implement this... If you design it correctly, it's simple. If not, well... :) Pete I wanted to try using Tree view to learn how to work with it.. thats why I used it :) connstatus is of type bool. I am storing my objects in an ArrayList. I did not know ArrayList was different from LinkedList... Since I would be updating my UI while changing the elements in the arraylist, I understand that I have to use synchronization. I shall try to implement what you suggested and get back if I have questions. Thanks once again! > > Thank you very much for the pointers to get started. Regarding > > synchronization of shared objects, currently I have a user defined class [quoted text clipped - 72 lines] > > Pete Hi, Thanks for your constant help. I am now having trouble sometimes and im guessing it has to do with mul-threading. As suggested by you, I used the "lock" synchronization mechanism to manage a shared arraylist object. I have a UI Thread that continuously takes the information from that shared object and depending on the node(IP Address) selected, it chooses that info from the shared arraylist object and it displays that on the UI. Its accessing the shared arraylist within the "lock" enclosed block. Within the the "lock" block I call a Control.Invoke(delegate) method that actually updates the UI. So should I encode that also withing "lock" block? At the same time when I try to click connect on that node, the connect event also tries to access that shared arraylist to update its contents. I have that code included within "lock". Looks like everything is synchronized. But When I try to click "Connect" while the UI thread is running, the application hangs sometimes. Sometimes it doesnt. I am guessing there is some kind of deadlock that happens randomly. Wantto make sure my code works right. Here are the two methods that r trying to access the shared resource . private void UIThread() { while (!AppShutDown) { lock (locker) { if (NRArrayList.Count > 0) //Checks if tree has any nodes { this.Invoke(this.m_DelegateUpdateUI); //calls the UI update //function that does not have a lock } } Thread.Sleep(500); } this.Invoke(new EventHandler(this.CloseMe)); } private void DelegateUpdateUI() { if (!AppShutDown) { if (IPTreeView.SelectedNode.Index >= 0) { foreach (NetworkRecorder nr in NRArrayList) { if (IPTreeView.SelectedNode.Text == nr.recorderip.ToString()) { if (nr.monitoring) { switch (nr.connstatus) { case true: //update text on the form lblstatus.text="running"; break; case false: lblConnStatus.Text = "Not Running"; break; } } else { lblConnStatus.Text = "None"; } } } } } } The question is , do I need to enclose the code in the DelegateUpdateUI using "lock"?? Now simultaneouslt the connect event when fired does the following: private void mnuConnect_Click(object sender, EventArgs e) { lock (locker) { foreach (NetworkRecorder nr in NRArrayList) { if (nr.recorderip.ToString() == IPTreeView.SelectedNode.Text) { this.numThreads++; nr.client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); nr.remoteEP = new IPEndPoint(nr.recorderip, 8001); nr.monitoring = true; nr.client.BeginConnect(nr.remoteEP, new AsyncCallback(ConnectCallback), nr); break; } } } This calls Connect CallBack function which is also accessing the shared resource ultimately. private void ConnectCallback(IAsyncResult ar) { NetworkRecorder nr = (NetworkRecorder)ar.AsyncState; if ((nr.monitoring)&&!(AppShutDown)) { try { nr.client.EndConnect(ar); this.numThreads--; nr.connstatus = true; nr.state = new StateObject(); nr.state.workSocket = nr.client; // Begin receiving the data from the remote device. this.numThreads++; nr.client.BeginReceive(nr.state.buffer, 0, nr.state.buffer.Length, 0, new AsyncCallback(ReceiveCallback), nr); } catch (Exception) { nr.connstatus = false; nr.client.Close(); nr.client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); Thread.Sleep(500); nr.client.BeginConnect(nr.remoteEP, new AsyncCallback(ConnectCallback), nr); } } else { nr.client.Close(); if (AppShutDown) this.Invoke(new EventHandler(this.CloseMe)); if (nr.monitoring) this.numThreads--; } } Now should ConnectCallBack also have "lock" since its also accessing the shared resource. Please give me an idea how to synchronize the shared arraylist object among all these methods. Thanks a lot!! > Hi, > > Thanks for your constant help. I am now having trouble sometimes and im > guessing it has to do with mul-threading. As suggested by you, I used the > "lock" synchronization mechanism to manage a shared arraylist object. After looking at the code you posted, I'm not sure you need the lock. > I have a UI Thread that continuously takes the information from that shared > object and depending on the node(IP Address) selected, it chooses that info > from the shared arraylist object and it displays that on the UI. IMHO, this is the part of your design that could use the most help. In particular, you've got this thread that's polling the data every half second. However, your network i/o code already knows when changes happen. So it would be better to have the network i/o code initiate the UI update. Doing this will simplify synchronization, and get rid of the extra thread completely. > Its > accessing the shared arraylist within the "lock" enclosed block. Within the > the "lock" block I call a Control.Invoke(delegate) method that actually > updates the UI. So should I encode that also withing "lock" block? My experience has been that using Control.Invoke() with some other synchronization object is a recipe for deadlock. As you seem to have found. :) > At the same time when I try to click connect on that node, the connect event > also tries to access that shared arraylist to update its contents. I have [quoted text clipped - 3 lines] > while the UI thread is running, the application hangs sometimes. Sometimes it > doesnt. I am guessing there is some kind of deadlock that happens randomly. You guess correctly, I think. Invoke() will not return until the Control instance's thread has had a chance to retrieve the invocation and execute it. When you call Invoke(), you have already obtained the lock for your synchronization object, which prevents the Control instance's thread from getting it. If the Control instance's thread attempts to get the locking object at this point, it will block. It won't be able to do anything until it receives the lock, but it's not going to get the lock because the other thread won't be able to do anything until the Invoke() completes, which won't happen because the Control instance's thread is blocked. Deadlock. However, as I mention above, I don't believe you need the extra thread anyway. > [code snipped] > The question is , do I need to enclose the code in the DelegateUpdateUI > using "lock"?? Not exactly. That is, since you are using Control.Invoke(), that code will be executed on the same thread that is normally using the ArrayList (I presume...that appears to be your design). However, before you call Invoke(), you do access the ArrayList.Count property. Also, since you do something based on the value of this property, it would probably be unwise to let go of the lock until you are done doing what depends on the lock. But the fact is, you don't really need that extra thread anyway, so the locking issue (at least in that part of your code) just goes away. If you really must poll, you should just set a Forms.Timer object to go off every 500ms and do the UI update in the event raised by the Timer. This Timer will execute on the main thread and so there won't be any synchronization issues with respect to the ArrayList itself. Even better would be to follow my suggestion and do away with the polling altogether. > [more code snipped] > Now should ConnectCallBack also have "lock" since its also accessing the > shared resource. > > Please give me an idea how to synchronize the shared arraylist object among > all these methods. In addition to what I already wrote: It appears to me that the only data that actually needs to be shared between threads is the nr.connstatus value. However, since the network i/o code already knows when this value changes, there doesn't appear to be a real need for the non-network i/o code to actually access that value. It would be better for the network i/o code to simply use Invoke() (or even BeginInvoke()) to update the UI in response to changes in the value. You don't even need for the invoked delegate to access the NetworkRecorder class directly at all. Rather than create a situation where you need to synchronize access to an instance of NetworkRecorder (which would be required if you use BeginInvoke(), which is otherwise a better way to go), just have the network i/o report a change by passing to the delegate a string representing the IP address, and the true/false value of the connstatus value. That's all the UI really needs to know, and by limiting the data passed to those copies of the actual data in the NetworkRecorder, you avoid the synchronization issue altogether. One last thing: I notice you keep a counter that looks like it's supposed to track active threads. However, you should be aware that the BeginXXX() methods for the Socket class do not normally actually create a new thread. Your counter is really counting outstanding i/o requests...no threads are active until an i/o completes, and you could have a single thread wind up processing a sequence of simultaneously created i/o requests. There's not actually a 1-to-1 mapping between i/o requests and threads. Whether this is actually important for your application or not, I don't know. It looks like it might not be, but I figured I'd mention it anyway. Pete > It appears to me that the only data that actually needs to be shared > between threads is the nr.connstatus value. However, since the network [quoted text clipped - 13 lines] > and by limiting the data passed to those copies of the actual data in > the NetworkRecorder, you avoid the synchronization issue altogether. Your idea sounds really good. I wouldnt need to worry about synchronization issues... Here is what I udnerstand, please let me know if its correct. I need to update the UI with various values and not only conn status. However, I get the idea that I could pass all the info of the entire object off as an arguement( copying those values to a local object of NetworkRecorder class) to the updateUI delegate from the network i/o code. and in the updateUI delegate function i check if that particular ip address is selected in the tree view or not. Based on that I choose to update the UI or not. While I try this, Please let me know if this implementation seems to be fine. > One last thing: I notice you keep a counter that looks like it's > supposed to track active threads. However, you should be aware that the [quoted text clipped - 4 lines] > created i/o requests. There's not actually a 1-to-1 mapping between i/o > requests and threads. I can understand what you are saying regarding the thread counter, but in my case, irrespective of the connection being established or not, I am calling the connectCallBack function in a loop until the connection successfully establishes because I need to continuously monitor the remote application. Remember I had asked you bout this and based on your suggeston, then I had decided to enclose the endconnect function in the try block and if the connection failed, the catch block would close the client, update the UI with the status and try the whole process again. Thats why whether the connection takes place or not , I will definitely have a thread running which is continuously trying to connect. After the connection gets establishes, I decrement the number of threads counter because logically the connection thread ends and receive thread starts. And increment the thread counter when it starts receiving. And the same logic here. I had to keep track of number of threads because at the end I am implementing a function which does a graceful shutdown of all the threads(where it needs to know the thread count) before it closes the application. Please let me know if you find any flaws in this design or if you have any better suggestions on implementing this. Thanks very much. I am learning a lot from your suggestions! > [...] > Here is what I udnerstand, please let me know if its correct. [quoted text clipped - 8 lines] > > While I try this, Please let me know if this implementation seems to be fine. As far as I understand it, it seems mostly fine. I'm not sure that making a whole new copy of your NetworkRecorder instance is correct. Don't you have references to other objects in there? Like your Socket instance? I think it might make more sense to create a status class that contains the specific data relevant to the update. Then your NetworkRecorder class can contain an instance of that status class, and when it needs to invoke the delegate for updating the UI, it can clone that (you can implement ICloning easily by just calling MemberwiseClone() in the Clone() method of your new class. Alternatively, you could just pass all of the values for the status as parameters for the invoke, by putting them in an object[] passed to the Invoke() method. The key is to not pass references that may be in use by the thread, like the Socket instance. You can say now "well, I just won't touch those parts", but it's much better to just design the code so it's not possible to in the first place. > I can understand what you are saying regarding the thread counter, I don't think you do. > but in my > case, irrespective of the connection being established or not, I am calling > the connectCallBack function in a loop until the connection successfully > establishes [...] Thats why whether the connection > takes place or not , I will definitely have a thread running which is > continuously trying to connect. No, you do not have a thread running. That's my point. Calling BeginConnect() does not start a new thread. It creates an i/o request. If and when it completes, the i/o request is dequeued by an already-existing thread. That same thread could handle any number of other i/o requests as well. > After the connection gets establishes, I decrement the number of threads > counter because logically the connection thread ends and receive thread > starts. And increment the thread counter when it starts receiving. And the > same logic here. And you have the same error in your logic here too. Calling BeginReceive() does not start a new thread. It only creates an i/o request. As with the BeginConnect() case, that i/o request will eventually be dequeued and processed by an already-existing thread. > I had to keep track of number of threads because at the end I am > implementing a function which does a graceful shutdown of all the > threads(where it needs to know the thread count) before it closes the > application. Well, you're not tracking the number of threads. You're tracking the number of i/o requests. Which may be fine, and desirable for your design, but you should understand that you aren't tracking threads. Pete > As far as I understand it, it seems mostly fine. I'm not sure that > making a whole new copy of your NetworkRecorder instance is correct. [quoted text clipped - 11 lines] > parameters for the invoke, by putting them in an object[] passed to the > Invoke() method. > The key is to not pass references that may be in use by the thread, like > the Socket instance. You can say now "well, I just won't touch those > parts", but it's much better to just design the code so it's not > possible to in the first place. If I clone an object ,for example the Status Class object that contains string types (which are reference types) will I be able to perform a deep copy of those objects? After implementing memberwiseclone, will i not be referring to the same objects as before or will this do a deep copy of the object. If it does a shallow copy then will it not have synchronization issues when i am accessing the object but at the same time updating it? > Well, you're not tracking the number of threads. You're tracking the > number of i/o requests. Which may be fine, and desirable for your > design, but you should understand that you aren't tracking threads. Now I understand what you are trying to say. But to do a clean shutdown of all the pending threads,sockets how should i keep track of them and how should i ensure all of them end properly before the application shuts down. >> The key is to not pass references that may be in use by the thread, like >> the Socket instance. You can say now "well, I just won't touch those [quoted text clipped - 4 lines] > string types (which are reference types) will I be able to perform a deep > copy of those objects? I should clarify: it's not reference types per se that are a problem. It's mutable reference types, that might be changed by one thread but accessed by another, or are specifically not thread-safe. As far as performing a deep copy goes, that depends on the type. But, for instance, I would not expect a deep copy that includes a Socket instance to be useful or practical. IMHO, it is better to create a data structure that contains just the data you need, in a way that has copied the data so that no other thread will be modifying. Note too my previous point that you don't really need a separate data structure at all. You _can_ just pass the values needed for the UI update as individual parameters to the method if you like. As long as the number of values is relatively small, this would be my preferred approach. If passing the values as parameters, the same issues apply with respect to reference types, but at least you don't need to worry about creating a whole new class, or of implementing ICloneable for that class (neither are difficult things, but if you don't have to and there's not a significant benefit from doing so, why bother?). > After implementing memberwiseclone, will i not be > referring to the same objects as before or will this do a deep copy of the > object. If it does a shallow copy then will it not have synchronization > issues when i am accessing the object but at the same time updating it? Yes, a memberwise-clone is referring to the same objects. If they are strings, this isn't a problem. That was my point. Sorry that I didn't make that clear enough; I admit my previous post was a little vague on that. The point here is that the data structure won't contain things that _will_ be a problem if they are simply cloned by MemberwiseClone. They should either be value types, or if they are reference types they should be reference types that are thread-safe and not mutable by other threads. The main example of something that would _not_ be in this data structure is the Socket reference; it shouldn't be needed for the UI update, and is exactly the sort of thing that can mutate after the fact, causing potential confusion between threads should they happen to try to use the object at the same time. >> Well, you're not tracking the number of threads. You're tracking the >> number of i/o requests. Which may be fine, and desirable for your [quoted text clipped - 3 lines] > all the pending threads,sockets how should i keep track of them and how > should i ensure all of them end properly before the application shuts down. I don't think that the counting mechanism you're using is necessarily wrong. If it's doing what you want, then that's fine. My only point was with respect to the mistaken impression you appeared to have as to what it was you were counting. For the moment, the question of whether you are counting threads or counting i/o requests may not matter. But if you come back later to the code, it might and at that point having the correct knowledge of what the counter is actually counting could be important. So, the main change I'd suggest is to rename the counter to that it more correctly reflects what it's counting. If the implementation otherwise works for you, then you shouldn't have to change anything else. Pete > I should clarify: it's not reference types per se that are a problem. > It's mutable reference types, that might be changed by one thread but [quoted text clipped - 5 lines] > structure that contains just the data you need, in a way that has copied > the data so that no other thread will be modifying. > Yes, a memberwise-clone is referring to the same objects. If they are > strings, this isn't a problem. That was my point. Sorry that I didn't [quoted text clipped - 10 lines] > potential confusion between threads should they happen to try to use the > object at the same time. I was under the impression that we need to synchronize access to all shared variables between the threads because the examples i saw that generally books giv for synchronization is generally a integer counter that is shared between different threads. If all we have to worry about is mutable reference variables and not value types, then why should someone worry about a integer counter being accessed by multiple threads? Or did I still get the concept wrong??!! I am so confused... Why a string value or an integer value does not need synchronized access by multiple threads? For example if one thread say mythread is making changes to the string variable say str which is immutable, its basically creating another string with the modified value and re-assigning the new reference to str. now when we call invoke and pass str as a parameter.... are we not passing the same reference. So basically the delegate would have the reference to the same string which was modified in mythread. So if the delegate concurrently accesses the str value while mythread is acessing it, why wouldnt it be a problem? Similar thing for the integer counter...? > I was under the impression that we need to synchronize access to all shared > variables between the threads because the examples i saw that generally books > giv for synchronization is generally a integer counter that is shared between > different threads. If all we have to worry about is mutable reference > variables and not value types, then why should someone worry about a integer > counter being accessed by multiple threads? I am sorry if I'm not explaining this well. Let me try again... The issue here is whether the variable is a) mutable, and b) that mutation can be seen by multiple threads. Value types can be mutable as well, but what I'm saying is that if you pass a copy of the value type, then the original can mutate and it doesn't matter. Passing a copy of a reference type variable just passes the reference, and so mutations in the instance can still be seen by multiple threads. > Or did I still get the concept wrong??!! I am so confused... Why a string > value or an integer value does not need synchronized access by multiple > threads? The string type is a reference type, but a string instance is immutable and the string class is thread-safe. So, you can for all intents and purposes treat it in the same way you'd treat a copy of a value type. That is, you can pass the value of a string variable without any problem to another thread, because even though it's a reference type, you are guaranteed that the instance's properties won't actually change. The same is not true of the string variable itself. That is, if you have a string variable and it itself is being accessed by multiple threads, then access to the _variable_ needs to be synchronized. But in that case, the variable itself is mutable and that's why you need the synchronization. Likewise a mutable value type variable (such as the integer counter you see in the examples you're talking about); as long as you pass the _value_ of that variable, you're fine. It's when the variable itself can be accessed by mu |
Enable EMail Alerts
Start New Thread
Reply to this Message