|
Hi Ian,
Ian write ...
However, that's not all... You completely fail to address the points in the next section:
>>However, the place where the value of connectId is then used
>>- the ReadSocket method on another thread - is a hostage to
>>fortune - what if the WaitingForClient() method manages to
>>accept a second incoming request before the second one gets
>>as far as reading connectId? (Also, ReadSocket implies
>> connectId is a 'long' - why? Its value is constrained to be
>> 10000 or lower, so having a 64 bit value is just pointless.
>> And copying one long into another like that is not guaranteed
>> to be thread-safe on 32 bit systems.)
-- Beacause WaitingClient using while-loop to handle the new
-- connection, After accepted new connection
-- and processed, then can accept another new coming connection.
-- The ReadSocket thread only using conectId when it started.
-- After the thread is started, then WaitingClient can start to
-- accept new connection. The satuation what you concern will be
-- not happened.
>>The ReadSocket method updates a ListView directly. This
>>is illegal - you are not supposed to call methods on a
>>Control on any thread other than the one that created it.
>>These calls should be done via the Control.Invoke method
>>or Control.BeginInvoke. Interestingly LoadThread gets
>>this right when updating the ListView.)
-- You are wrong completely and should learn something from here.
-- This is the best solution to update the control from the
-- background thread!
Well I can back my statement up with authoritative sources: I quote from Microsoft's documentation:
So the very thing you recommend (updating controls from background threads) is explicitly not allowed, according to Mark Boulter. Since he is a developer working for the part of Microsoft who produced this technology, I would very much like to know why it is that you claim to know more than him about how to use Windows Forms Controls.
....
-- Please goto How Do I ... Common tasks, Windows Forms: and
-- Manipulate a control from a background thread?
-- The article: Making procedure calls across thread boundaries
-- and sample code that "demonstrates how to create a background
-- thread that uses a MethodInvoker to update a ProgressBar
-- control at regular intervals:"
-- This is not some personal opinion. It microsoft published code
-- with the Beta 2 DVD. That represents Microsoft. It is created
-- by Microsoft expert. You may think it is terrible sample.
-- Publish your idea on it. Let me know what do you think about
-- the sample. I use the same way that the sample did. I use
-- asynchronous invocation to make everything easy. Why do you
-- force me to use synchronous call that you should get the
-- instance of the form and instance of the listview control from
-- background thread that is extral work when you use
-- Control.Invoke.
So even with error handling built in, this code is entirely inadequate for the problem at hand. There is absolutely no sense in which it is real time - it is limited by the slowness of the slowest client, and there is no bound on how slow that might be. The only way to deal with this is to use a multi-threaded send of some kind. I would fire up a watchdog thread which checks to make sure the sending thread hasn't blocked for more than, say, a second, and to spin up a new thread if it has. Or it might be possible to build a satisfactory solution around the system thread pool. A third approach might be to use non-blocking IO, which would probably allow the most efficient solution, but would be the most complex approach.
-- I like this comments. But This program just a small demo, you
-- can say it is a toy. You can't expect it to solve all the
-- problems in the TCP/IP world.
-- Thanks any way. I holp reader can learn some from these.
Jibin Pan
|
|
|
|
|
-- Beacause WaitingClient using while-loop to handle the new
-- connection, After accepted new connection
-- and processed, then can accept another new coming connection.
-- The ReadSocket thread only using conectId when it started.
-- After the thread is started, then WaitingClient can start to
-- accept new connection. The satuation what you concern will be
-- not happened.
Here is a sequence of events that will cause exactly the problem I describe to occur:
(1) A client connection request comes in
(2) In WaitingForClient, tcpLsn.AcceptSocket returns.
(3) WaitingForClient increments connectId, to (say) 2
(4) Another client connection request comes in. (This won't be handled yet, because - it will only get handled when the main thread loops back round. However, the request will sit in the OS's queue.)
(5) WaitingForClient checks ID 2 is not in use right now
(6) WaitingForClient calls Thread.Start to run the new thread
(7) The OS decides that although it could start running the new thread, it realises that the main thread hasn't used up its current quanta and so it allows the main thread to carry on running.
(8) The main thread loops round, calls tcpLsn.AcceptSocket which returns immediately because of the second client connection request that came in earlier.
(9) The main thread increments connectId to 3.
(10) The main thread kicks off another thread
(11) The main thread loops round again, and this time it blocks in AcceptSocket
(12) The OS decides to give the next thread a go. It starts up, reads connectId and gets the value 3. So it sets its realId value to 3.
(13) The OS scheduler switches to the other thread that got created. It starts up, reads connectId and also gets the value 3.
So there are now two threads running, both of which think their ID is 3. Surely this is a bug?
Do you agree with this analysis? As far as I can tell, this is not only possible, it's positively likely to happen. It's not unusual for a single listening socket to have multiple connection requests queue up faster than they are handled. This is likely to happen when everyone connects to your server first thing in the morning. And the scheduler will very likely behave in the manner I describe - it tries to minimise the number of thread switches, so the main thread will almost certainly carry on running after it has called Thread.Start, and the threads thus launched won't actually begin their work until the main thread runs out of things to do.
There are variations on the theme. If everything happens the same as before up to 8, consider the following alternative sequence after that point:
(9) The OS switches to the other thread.
(10) The other thread starts to read connectId. Remember that reading from a long is not an atomic operation.
(11) The OS decides to switch back to the main thread. The main thread calls Interlocked.Increment to increment connectId. This will succeed - it has no way of knowing that there was another thread trying to read from connectId at the time.
(12) The OS switches back to the other thread, which carries on doing its read.
So the launched thread's value of realId will be corrupt. It has been interrupted by a thread switch. (That can definitely happen - read the CLR spec.) A 64 bit read operation which has been split in this way has an undefined value, so realId could be absolutely anything.
In practice this second scenario is not going to happen very often. But that's no excuse - threading hazards that don't happen often still happen, they just happen once every 6 months or so. In practice I would be surprised if it ever caused a real problem, simply because your value never goes above 10,000 so the top half of the long is always 0, so on current architectures you will happen to get the right value anyway. So you are relying on undocumented behaviour. But what I still don't understand is why on earth you chose to make this a long! Just make it an int, and this particular problem goes away.
However, the other problem - the problem of 2 different threads both getting the same ID won't be solved by changing to an int. That's a more fundamental problem with your design, and require more radical changes.
-- Please goto How Do I ... Common tasks, Windows Forms: and
-- Manipulate a control from a background thread?
-- The article: Making procedure calls across thread boundaries
-- and sample code that "demonstrates how to create a background
-- thread that uses a MethodInvoker to update a ProgressBar
-- control at regular intervals:"
I think we're talking about different parts of your code here. You're talking about StartReceive, where it passes a MethodInvoker to this.BeginInvoke, yes? That code is correct, although I think it would be better to call this.Invoke. (That will still marshal the call to the correct thread, as required. It will also block until UpdateListView has finished, which is what your comments say you want.)
That's not the code that I'm talking about. I'm talking about the code in ReadSocket.
In ReadSocket, you have the following lines of code:
this.listView2.Items.Add(newItem);
ind=this.listView2.Items.IndexOf(newItem);
and also
this.listView2.Items[ind].ImageIndex=1;
The ReadSocket method is not running on the UI thread. The reason I say that is that you also have this line of code:
Thread td = new Thread(new ThreadStart(ReadSocket));
So, the ReadSocket method doesn't run on the main thread, but it does call methods and use properties on listView2. This is wrong. You should be using this.Invoke or this.BeginInvoke here just like you do in your StartReceive method.
So, I wasn't complaining about StartReceive, or UpdateListView. These use the correct technique. I was complaining about the ReadSocket method. This is why I said in my original message "The ReadSocket method updates a ListView directly. This is illegal"
-- I like this comments. But This program just a small demo, you
-- can say it is a toy. You can't expect it to solve all the
-- problems in the TCP/IP world.
No, but I think it's somewhat misleading. You have given this article the title "Real Time TCP/IP using C#", and your example is all based around stock prices. There is nothing about this code that can be described as "Real Time" - it makes no guarantees about timing as it can be brought down by a single client. It is very definitely not a sound basis for a real time stock price update system.
So not only does it not solve all the problems in the TCP/IP world, I don't believe it even solves the problem that the title suggests it solves.
--
Ian Griffiths
DevelopMentor
|
|
|
|
|
-- I have to make some claim again. You didn't understand the
-- codes and your comments will mislead the reader.
For following comments:
>>The CloseTheThread mysteriously aborts the thread.
>> This is completely pointless, since the thread was
>> about return out of its starting function anyway,
>> at which point the system would have cleaned it up.
---- execute the Abort thread function when you don’t need the
---- thread. It is better than you depend on system to do it for
---- you. If the system does not do it for you, what happen …?
If the system does not do it for you then there is a bug in the system. So you are writing code that presumes that at some point Microsoft will release a version of the CLR which has a bug wherebye threads don't terminate when they should. Don't you think that this is a little paranoid?
As soon as the main function in a thread exits, the thread will terminate. It's not like it needs to hang around and wait for garbage collection. Aborting the thread will, if anything, slow things down, since forcibly aborting a thread is more expensive than letting it terminate naturally.
Normally you would only abort a thread from another thread. I.e. the purpose of the Abort method is to allow one thread to kill off another one. There is rarely any need for a thread to kill itself off like this. (In your case there is definitely no need, since it is easy for the threads to simply exit.)
-- First, I don't like your idea that you don't do the clean job
-- and let the operating system to do it for you. If the system
-- have a bug then your program have the bug. You can expect the
-- system is bug free, otherwise Microsoft don't need release
-- some server packages and create new operating systems.
-- Second, ReadSocket() check the client information to
-- determine the thread is needed or not and close the thread
-- when the client is logoff. The operating system can't do it
-- for you. Do you want to keep these threads alive until you
-- close the main thread?
-- I find the link for the update control from background
-- thread on the Web. I have said that "you can learn something
-- from here".
-- is not from me, is from Microsoft. http://samples.gotdotnet.com/QuickStart/aspplus/default.aspx?url=/quickstart/howto/doc/WinForms/WinFormsThreadMarshalling.aspx
-- Please see the code that is same as I did.
|
|
|
|
|
-- First, I don't like your idea that you don't do the clean job
-- and let the operating system to do it for you. If the system
-- have a bug then your program have the bug. You can expect the
-- system is bug free,
Of course no system is bug free, but do you have any reason to suppose that a bug of the nature you describe will emerge? Right now, threads terminate when you return from their ThreadStart function. It would be a pretty serious bug if this stopped working, because it would mean that most programs that launch threads would suddenly start to suffer from serious performance problems as the number of threads in the process stacked up uncontrollably.
So if Microsoft ever *did* manage to introduce such a bug into a future version by mistake, it would break a huge amount of code. Yours is the only program I have ever seen that anticipates such a bug in the runtime, so the vast majority of multithreaded programs would break. This suggests that such a release wouldn't make it through beta testing.
So don't you think you're being a bit paranoid? And even if you do think that it's appropriate to presume that such random and major bugs will appear in future releases, why are you any more confident that aborting the thread will work? If you don't have any faith that Microsoft won't break System.Thread in such a way that it doesn't terminate threads when they exit, why do you have any more faith that System.Thread.Abort won't also be broken in a future release?
Working around known bugs is fine, and standard practice. Putting in workarounds for bugs that have never existed, and which you have no good reason to suppose will *ever* exist is pointless, because if you take that approach to its logical conclusion, you'll never trust the platform to do anything at all!
-- I find the link for the update control from background
-- thread on the Web. I have said that "you can learn something
-- from here".
-- is not from me, is from Microsoft. http://samples.gotdotnet.com/QuickStart/aspplus/default.aspx?url=/quickstart/howto/doc/WinForms/WinFormsThreadMarshalling.aspx
-- Please see the code that is same as I did.
Following this link, the first thing it says is:
"Windows Forms controls can only execute on the thread on which they were created"
This is precisely my point. I'll say it one more time:
The problem is in your ReadSocket function.
The code you have in the ReadSocket function does NOT work in the way recommended by that article.
PLEASE GO AND RE-READ THE CODE IN THE ReadSocket FUNCTION. I know that your code does it correctly in the StartReceive function and the LoadThread function - that code does indeed do the same thing as the article. I'm not complaining about the StartReceive function or the LoadThread function. I'm complaining about the ReadSocket function, which does several things to listView2, despite running on a non-UI thread. The code in the ReadSocket function does NOT follow the recommendations (i.e. it doesn't use Control.Invoke), which is what I'm complaining about.
There is not a single call to Invoke or BeginInvoke inside the ReadSocket method, nor do you use MethodInvoker in there.
I don't know how many more ways I can find to say this. Your code gets it right in two places (StartReceive and LoadThread) and wrong in one place: ReadSocket
As a side issue, I think you have misunderstood why Microsoft's example calls Thread.Sleep(500) after BeginInvoke - it is only doing that to fake up some 'work'. That's in their example because their worker thread doesn't do anything real, so it just sleeps instead. You didn't need to copy that bit across. It doesn't make your code actively wrong as such, but it does mean that it pointlessly wastes some time.
--
Ian Griffiths
DevelopMentor
|
|
|
|
|
I think Ian Griffiths is a total tit... No half decent coder types so much crap if he's such a mentor then where thy code?
Don't need many brain threads on that! Mentor My Arse? Minetor More Like...
The IT bizz is full of Dome Heads Like This Guy Thats Why It Sucks.
Thank 4 your example anyhow.;P ;) (( :->
|
|
|
|
|
IIan
Can you point me to an article of how to write a TCPClient/TCPServer?
|
|
|
|
|
|
High,
i read a message, that you worked on a socket server in C# Forms.
I need to do a socket server which communicates with up to 100 client. they exchange only a few bytes.
What i want is a Form an which i can put the communication in Listviews and store thme into the sqlserver via ado.net. Is there any sample in the www for this or parts of this?
I only find Console.Apps or the sample in codeprojct which may be not save for 24/7 working environment.
Any info ?
Thanks for your help,
christian
|
|
|
|
|
Hi,
listening at 0.0.0.0:8002 means that your program is listening to ANY connected attempt to port 8002 (from RAS, LAN, ...)
If your specify an IP adress then only requests that are received from the specified adapter are accepted. The your output will show the specified ip adress.
Ciao Matthias Mann
|
|
|
|
|
Now, I didn't read your article very carefully, but it seems to me that every writing to socketHolder and threadHolder should be "locked"
I vote pro drink
|
|
|
|
|
Please add WTcpClient to your zipped file. Thanks.
|
|
|
|
|