Thread Problem

J

JC

Hi People,

Please I need your help.

This code run a thread ok but Not close later.

thanks...

private void RunServer(int aPortNumber)
{
_listener = new TcpListener(IPAddress.Any, aPortNumber);
_listener.Start();

_ServerThread = new Thread(delegate()
{
AcceptClients();
}
);
_ServerThread.Start();
}

public void Close()
{

_ThreadRun = false;

}


public void AcceptClients()
{
while (_ThreadRun)
using (TcpClient client = _listener.AcceptTcpClient())
{
if (client.Connected)
{
NetworkStream stream = client.GetStream();
byte[] data = new byte[1024];
stream.Read(data, 0, data.Length);
string request = Encoding.ASCII.GetString(data);

......
}

}
 
G

gbgeek

Hi JC,

I might be overlooking this but, how is the Close() method called? Is
that really an event method for a form closing or is it something else?
 
D

Dave Sexton

Hi,

That is because _listener.AcceptTcpClient() is blocking the thread. Setting
_ThreadRun to false isn't going to do anything until
_listener.AcceptTcpClient() returns.

Since there doesn't seem to be any error handling code in place, you won't
be able to end the background thread gracefully. Calling _listener.Stop()
will close the listener, causing AcceptTcpClient to stop blocking, however
an exception will be thrown. Setting _ThreadRun would be pointless in that
case, although it's possible that setting _ThreadRun may cause the loop to
exit if it's set after _listener.AcceptTcpClient() returns and before the
loop restarts, but you shouldn't count on that.

Try the following code instead:

while (true)
{
try
{
using (TcpClient client = _listener.AcceptTcpClient())
{
if (client.Connected)
{
NetworkStream stream = client.GetStream();
byte[] data = new byte[1024];
stream.Read(data, 0, data.Length);
string request = Encoding.ASCII.GetString(data);
}
}
}
catch (SocketException ex)
{
if (ex.SocketErrorCode == SocketError.Interrupted)
return;
else
throw;
}
}
 
T

ThunderMusic

hi,

I've always learned that relying on exception as a normal process is bad
practice when there is another way to go. And in this case, there is another
way to go : BeginAcceptTcpClient(...) Just call
_listener.BeginAcceptTcpClient(...) and everything will be fine... ;) if
you need help using it, go there :

http://msdn2.microsoft.com/en-us/library/system.net.sockets.tcplistener.beginaccepttcpclient.aspx

I hope it helps

ThunderMusic

Dave Sexton said:
Hi,

That is because _listener.AcceptTcpClient() is blocking the thread.
Setting _ThreadRun to false isn't going to do anything until
_listener.AcceptTcpClient() returns.

Since there doesn't seem to be any error handling code in place, you won't
be able to end the background thread gracefully. Calling _listener.Stop()
will close the listener, causing AcceptTcpClient to stop blocking, however
an exception will be thrown. Setting _ThreadRun would be pointless in
that case, although it's possible that setting _ThreadRun may cause the
loop to exit if it's set after _listener.AcceptTcpClient() returns and
before the loop restarts, but you shouldn't count on that.

Try the following code instead:

while (true)
{
try
{
using (TcpClient client = _listener.AcceptTcpClient())
{
if (client.Connected)
{
NetworkStream stream = client.GetStream();
byte[] data = new byte[1024];
stream.Read(data, 0, data.Length);
string request = Encoding.ASCII.GetString(data);
}
}
}
catch (SocketException ex)
{
if (ex.SocketErrorCode == SocketError.Interrupted)
return;
else
throw;
}
}

--
Dave Sexton

JC said:
Hi People,

Please I need your help.

This code run a thread ok but Not close later.

thanks...

private void RunServer(int aPortNumber)
{
_listener = new TcpListener(IPAddress.Any, aPortNumber);
_listener.Start();

_ServerThread = new Thread(delegate()
{
AcceptClients();
}
);
_ServerThread.Start();
}

public void Close()
{

_ThreadRun = false;

}


public void AcceptClients()
{
while (_ThreadRun)
using (TcpClient client = _listener.AcceptTcpClient())
{
if (client.Connected)
{
NetworkStream stream = client.GetStream();
byte[] data = new byte[1024];
stream.Read(data, 0, data.Length);
string request = Encoding.ASCII.GetString(data);

......
}

}
 
D

Dave Sexton

Hi,
I've always learned that relying on exception as a normal process is bad
practice when there is another way to go

If the code is going to work reliably then exception handling is a must, so
there is no harm in simply returning in the case of an exception that can be
handled.
And in this case, there is another way to go : BeginAcceptTcpClient(...)
Just call _listener.BeginAcceptTcpClient(...) and everything will be
fine... ;)

<snip>

BeginAcceptTcpClient will not help here.

The loop still has to block somehow, and if you call _listener.Stop() an
exception will still be thrown even though Begin* is called.

IAsyncResult result = _listener.BeginAcceptTcpClient(null, null);

// now, we're just blocking here instead of there ^
using (TcpClient client = _listener.EndAcceptTcpClient(result))
{
...
}

You could have a sub-loop that checks an EventWaitHandle and a variable such
as _ThreadRun, as in the OP, but the code will become more complex with no
real gain. And, exception handling code should still be implemented anyway.
 
B

Brian Gideon

JC,

Like Dave said TcpListener.AcceptTcpClient is your biggest problem.
But, even if you fix that the subtle memory barrier problem with
_ThreadRun still exists. You should wrap reads and writes to
_ThreadRun with a lock to ensure changes to it are visible in all
threads.

Brian
 
B

Brian Gideon

JC,

Like Dave said TcpListener.AcceptTcpClient is your biggest problem.
But, even if you fix that the subtle memory barrier problem with
_ThreadRun still exists. You should wrap reads and writes to
_ThreadRun with a lock to ensure changes to it are visible in all
threads.

Brian
 
H

Hernando Gisinger

Yo creo que todos tienen razón. También creo que deberían dejar de
escribir en inglés en un newsgroup en castellano.

Gracias.

Brian Gideon escribió:
JC,

Like Dave said TcpListener.AcceptTcpClient is your biggest problem.
But, even if you fix that the subtle memory barrier problem with
_ThreadRun still exists. You should wrap reads and writes to
_ThreadRun with a lock to ensure changes to it are visible in all
threads.

Brian
Hi People,

Please I need your help.

This code run a thread ok but Not close later.

thanks...

private void RunServer(int aPortNumber)
{
_listener = new TcpListener(IPAddress.Any, aPortNumber);
_listener.Start();

_ServerThread = new Thread(delegate()
{
AcceptClients();
}
);
_ServerThread.Start();
}

public void Close()
{

_ThreadRun = false;

}


public void AcceptClients()
{
while (_ThreadRun)
using (TcpClient client = _listener.AcceptTcpClient())
{
if (client.Connected)
{
NetworkStream stream = client.GetStream();
byte[] data = new byte[1024];
stream.Read(data, 0, data.Length);
string request = Encoding.ASCII.GetString(data);

......
}

}
 
D

Dave Sexton

Hi Brian,

Good point about volatility, however the variable is pointless anyway. It
won't allow the OP to control the background thread.

The only way to stop the _listener.AcceptTcpClient method from blocking is
to call _listener.Stop(), which will cause an exception to be thrown and the
variable won't be checked.

BeginAcceptTcpClient cannot be stopped gracefully either. Calling
_listener.Stop() will cause an exception to be thrown as well.

The OP should write code to handle this exception and exit the background
thread, as in my example.

--
Dave Sexton

Brian Gideon said:
JC,

Like Dave said TcpListener.AcceptTcpClient is your biggest problem.
But, even if you fix that the subtle memory barrier problem with
_ThreadRun still exists. You should wrap reads and writes to
_ThreadRun with a lock to ensure changes to it are visible in all
threads.

Brian
Hi People,

Please I need your help.

This code run a thread ok but Not close later.

thanks...

private void RunServer(int aPortNumber)
{
_listener = new TcpListener(IPAddress.Any, aPortNumber);
_listener.Start();

_ServerThread = new Thread(delegate()
{
AcceptClients();
}
);
_ServerThread.Start();
}

public void Close()
{

_ThreadRun = false;

}


public void AcceptClients()
{
while (_ThreadRun)
using (TcpClient client = _listener.AcceptTcpClient())
{
if (client.Connected)
{
NetworkStream stream = client.GetStream();
byte[] data = new byte[1024];
stream.Read(data, 0, data.Length);
string request = Encoding.ASCII.GetString(data);

......
}

}
 
T

ThunderMusic

yes, exception handling is a must to handle exceptional situations, but the
process should never be based on exception handling as you are advising
here... and yes there is harm because exception handling is very costy
resource-wise, so if you can avoid it, avoid it...

I really think you should read on asynchronous programming, it would really
help... I've done this kind of processing many times using
BeginAcceptTcpClient(...) without any problem when stopping my apps... I
don't have sample code at hand, but I know it worked fine... follow the
link I provided and you will most likely find what you need...

I hope it helps

ThunderMusic
 
D

Dave Sexton

Hi,
yes, exception handling is a must to handle exceptional situations, but
the process should never be based on exception handling as you are
advising here... and yes there is harm because exception handling is very
costy resource-wise, so if you can avoid it, avoid it...

Your recommendation works great for control-of-flow on a single thread (e.g,
don't use exceptions to control flow :), but it doesn't apply in this
situation.

My code doesn't process based on exceptions, it handles a situation that
cannot be avoided in a way that will perform better or be easier to read,
IMO. I'd much rather have the I/O completion thread block instead of
spinning a thread for an arbitrary number of milliseconds to periodically
check a wait handle or check a volatile variable, or having to lock and
unlock an object.

The exception will only be thrown once, indeterminately, so it is an
exceptional circumstance.

If the OP is planning on starting and stopping the listener repeatedly than
I might agree that a different solution would be in order, however I assume
that isn't the case.

The code is clean and should perform exceptionally well. (no pun intended
:)
I really think you should read on asynchronous programming, it would
really help... I've done this kind of processing many times using
BeginAcceptTcpClient(...) without any problem when stopping my apps... I
don't have sample code at hand, but I know it worked fine... follow the
link I provided and you will most likely find what you need...

I can assure you that a SocketException is thrown by the
EndAcceptTcpClient() method when you stop the listener on another thread.

In the 1.* frameworks you wouldn't have known there was an exception being
thrown when you disposed of the listener, while it was being used on a
background thread, so that might have been your problem.

Also, it doesn't make sense to use a ThreadPool thread from a background
thread in this scenario. It's just asking for dead locks in a service
application.
 
J

Jon Skeet [C# MVP]

ThunderMusic said:
yes, exception handling is a must to handle exceptional situations, but the
process should never be based on exception handling as you are advising
here... and yes there is harm because exception handling is very costy
resource-wise, so if you can avoid it, avoid it...

Exception handling is not "very costly" - the performance implications
are almost never a good reason to avoid it. If a situation is
exceptional, an exception is appropriate. Exceptions shouldn't be used
without careful consideration, but for much design reasons more than
performance.

Just out of interest, how expensive do you think exceptions actually
are? Do you think you'd notice a significant difference in performance
if your application threw a thousand exceptions per second? I don't. An
application which threw a thousand exceptions per second would be very
worrying in terms of *design*, but the performance implications of that
are minimal when running outside the debugger.

See http://www.pobox.com/~skeet/csharp/exceptions.html for more on this
(including actual figures).
 
D

Dave Sexton

Hi Jon,

Great article.

You mentioned .NET 2.0 Beta 2 in the article, so I tested your code on my
desktop with .NET 2.0 (current, with 3.0 installed as well):

3.4 Ghz Xeon w/hyperthreading enabled (1)

( No debugger attached )

Total time taken: 00:04:44.0312500
Exceptions per millisecond: 17

Still remarkably fast, contrary to popular belief, but I noticed something
peculiar: processor usage was steady at ~50% the entire time I ran the
program (~50% per virtual processor). I tried 4 times, once in release
configuration, and the results were consistent between 16 and 17 exceptions
per millisecond. 2 of the tests were executed while running background
applications that are somewhat processor-intensive and highly
memory-consuming and 2 weren't, but that didn't seem to have any effect on
the outcome.

Any thoughts?
 
J

Jon Skeet [C# MVP]

Dave Sexton said:
Great article.

You mentioned .NET 2.0 Beta 2 in the article, so I tested your code on my
desktop with .NET 2.0 (current, with 3.0 installed as well):

3.4 Ghz Xeon w/hyperthreading enabled (1)

( No debugger attached )

Total time taken: 00:04:44.0312500
Exceptions per millisecond: 17

Still remarkably fast, contrary to popular belief, but I noticed something
peculiar: processor usage was steady at ~50% the entire time I ran the
program (~50% per virtual processor). I tried 4 times, once in release
configuration, and the results were consistent between 16 and 17 exceptions
per millisecond. 2 of the tests were executed while running background
applications that are somewhat processor-intensive and highly
memory-consuming and 2 weren't, but that didn't seem to have any effect on
the outcome.

Any thoughts?

Interesting. I've just rerun the test myself, on the same laptop as
before, but this time with Vista (so .NET 2.0 and 3.0) - I can "only"
throw 21 exceptions per millisecond.

That's still quite a lot, but it's about 6 times slower than it was in
1.1! Definitely time to update the article - it means that throwing
1000 exceptions per second (the example I used before) *would* have a
somewhat significant performance hit. My overall point is still valid
though, I believe - the design issues which are suggested by an
application throwing lots of exceptions are more important than the
performance implications.
 
D

Dave Sexton

Hi Jon,

Interesting. I've just rerun the test myself, on the same laptop as
before, but this time with Vista (so .NET 2.0 and 3.0) - I can "only"
throw 21 exceptions per millisecond.

That's still quite a lot, but it's about 6 times slower than it was in
1.1! Definitely time to update the article - it means that throwing
1000 exceptions per second (the example I used before) *would* have a
somewhat significant performance hit. My overall point is still valid
though, I believe - the design issues which are suggested by an
application throwing lots of exceptions are more important than the
performance implications.

I agree. My opinion hasn't really changed in light of these new results.
An application throwing 1000 exceptions per second would still lead me to
believe that there is some sort of design issue or bug that needs to be
dealt with before considering any performance implications that it may have
on the system, but avoiding throwing exceptions because of concerns of
performance is being overly cautious.

As for the tests:

I tried NGen before running it once, just for fun, but the results were no
different.

Good idea to try it on Vista. I tried it myself and saw a slight (very
slight) performance increase but with some strange behavior in CPU usage.

I've got a second HDD in my computer so the remaining hardware, such as CPU,
was the same for these tests:

Windows Vista (TM) Beta 2
Evaluation copy: Build 5384
(Clean Install)

Total time taken: 00:04:25.8552000
Exceptions per millisecond: 18

(I had over 1GB RAM free during each of these tests and on the previous XP
tests, in cast that matters at all).

On a side note:

Just like the 4 tests that I ran on XP SP2, the average CPU usage on the 4
Vista tests was consistent at ~50%.

However, CPU usage was quite different between the two processors on the
Vista tests. (The 4 runs on XP were consistent at ~50/50, in terms of %
Avg. CPU usage).

(Remember, this is one hyperthreaded processor and both XP and Vista used
the same exact hardware except for HDDs)

Vista Beta 2 test results as viewed in the Task Manager Performance tab (4
distinct runs):

1. Sporattic, with other processes running.
(I then waited for the CPU to idle before running any further tests)

2. ~80/20, but with spikes every 30 seconds. Second CPU resembled an EKG.
3. Consistent at ~80/20.
4. Perfect square wave at ~60/40 and ~40/60, switching at about every 45
seconds.

The last 3 tests were run without any other GUI running. I wasn't even
moving the mouse :)
 
B

Brian Gideon

Dave said:
Hi Brian,

Good point about volatility, however the variable is pointless anyway. It
won't allow the OP to control the background thread.

The only way to stop the _listener.AcceptTcpClient method from blocking is
to call _listener.Stop(), which will cause an exception to be thrown and the
variable won't be checked.

BeginAcceptTcpClient cannot be stopped gracefully either. Calling
_listener.Stop() will cause an exception to be thrown as well.

The OP should write code to handle this exception and exit the background
thread, as in my example.

Dave,

Good point. It seems that cancelling socket operations is a common
question. In the past I've specifically seen people ask about
cancelling the BeginRead method on a Socket. It's not obvious that you
have to actually close the socket. I remember being confused about it
several years ago when I did my first socket application.

Brian
 
H

Hernando Gisinger

En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés
En un newsgroup en castellano no debo escribir en inglés

JC escribió:
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top