Question abut threads

[Markgoldin]

I am working on a program that creates a few TCP listenrs to accept data
from non .net processes.
Each listener listens in its own thread. I am using Thread.Sleep(1000) to
let other threads work.
Here is my code:
tcpl = new TcpListener(ipe);
TcpClient tcpc = tcpl.AcceptTcpClient();
while (true)
{
// get data
while (true)
{
// if contidion met break;
}
Thread.Sleep(1000) ;
}

Do I really need Thread.Sleep(1000) ? What would happend if I dont use it at
all?

Thanks

 

[Jon Skeet [C# MVP]]

I am working on a program that creates a few TCP listenrs to accept data
from non .net processes.
Each listener listens in its own thread. I am using Thread.Sleep(1000) to
let other threads work.
Here is my code:
tcpl = new TcpListener(ipe);
TcpClient tcpc = tcpl.AcceptTcpClient();
while (true)
{
    // get data
    while (true)
    {
        // if contidion met break;
    }
    Thread.Sleep(1000) ;

}

Do I really need Thread.Sleep(1000) ? What would happend if I dont use itat
all?

That depends on what happens in the loop. Are you actually likely to
be processor bound? Is your while loop a tight loop? If so, that's the
thing to fix.

It's unlikely that you should really be sleeping, but if you ever need
to wait for a condition to be met, you should probably be using an
Auto/ManualResetEvent or Monitor.Wait/Pulse.

Jon

 

[Ignacio Machin ( .NET/ C# MVP )]

I am working on a program that creates a few TCP listenrs to accept data
from non .net processes.
Each listener listens in its own thread. I am using Thread.Sleep(1000) to
let other threads work.

you should only do this if each listener listen in a different post.

Here is my code:
tcpl = new TcpListener(ipe);
TcpClient tcpc = tcpl.AcceptTcpClient();
while (true)
{
    // get data
    while (true)
    {
        // if contidion met break;
    }
    Thread.Sleep(1000) ;

}

Do I really need Thread.Sleep(1000) ? What would happend if I dont use itat
all?

why you use it in the first place??

Answer to this, are you using one port or multiple ports?
I can give you code in each case, it's as simple as using a SyncQueue.
and one thread per connection (no per listener)

 

[Ignacio Machin ( .NET/ C# MVP )]

I believe Ignacio has a one-letter type here: "post" should be "port".  
Hopefully that was clear from the rest of his message, but if not this  
should clear it up.  

Sorry for the mistake, I also noticed a couple of words in spanish in
another post, I've been without coffee the entire morning, not good
for health

I don't find the type "SyncQueue" in the MSDN documentation, so I don't  
know what Ignacio is getting at.  That said, I'll suggest that there are  
really only two viable approaches to TCP network i/o in .NET:

There is none, but you have Queue.Synchronized method:
Returns a Queue wrapper that is synchronized (thread safe).

Seet his example:
Queue connectionQueue;
protected override void OnStart(string[] args)
{
listenerThread = new Thread( new ThreadStart( ListenerMethod));
listenerThread.Start();
}
protected void ListenerMethod()

{
Thread workingthread;
Queue unsyncq = new Queue();
connectionQueue = Queue.Synchronized( unsyncq);
TcpClient socket;
TcpListener listener = new TcpListener( 1212);

listener.Start();
while( true)
{
socket = listener.AcceptTcpClient();
connectionQueue.Enqueue( socket);
workingthread = new Thread( new
ThreadStart( TheConnectionHandler));
workingthread.Start();
}
}
public void TheConnectionHandler()
{
TcpClient socket= (TcpClient)connectionQueue.Dequeue();
//use the socket
}

 

[William Stacey [C# MVP]]

Instead of using threads to keep client state, you may want to look into
using async comm. The fx3.5 has some improvements in socket class that may
help:
http://msdn.microsoft.com/en-us/magazine/cc163356.aspx
--wjs

I am working on a program that creates a few TCP listenrs to accept data
from non .net processes.
Each listener listens in its own thread. I am using Thread.Sleep(1000) to
let other threads work.
Here is my code:
tcpl = new TcpListener(ipe);
TcpClient tcpc = tcpl.AcceptTcpClient();
while (true)
{
// get data
while (true)
{

....

 

[Ignacio Machin ( .NET/ C# MVP )]

[...]

I don't find the type "SyncQueue" in the MSDN documentation, so I don't  
know what Ignacio is getting at.  That said, I'll suggest that there 
are  
really only two viable approaches to TCP network i/o in .NET:

There is none, but you have Queue.Synchronized  method:
Returns a Queue wrapper that is synchronized (thread safe).

Seet his example:

All due respect and no offense intended, but...  yuck!  

You're using the queue just as a way of passing data from one thread to  
another?  What's wrong with using ParameterizedThreadStart?  For example:

No offense taken

You are right, you can use ParameterizedThreadStart . The thing is
that that piece of code is from 1.1 era , you had no
ParameterizedThreadStart back then
This is the original post:

http://groups.google.com/group/micr..."+"listener.AcceptTcpClient"#f6eabe586c0924e0

Just recently I updated it to 2005 but the code is the same, it works,
so why change it ?

     protected void ListenerMethod()
     {
         TcpListener listener = new TcpListener(1212);

         listener.Start();
         while (true)
         {
             TcpClient client = listener.AcceptTcpClient();

             new Thread(delegate { TheConnectionHandler(client) }).Start();
         }
     }

     protected void TheConnectionHandler(TcpClient client)
     {
         //...
     }

That would be the best solution

 

[Ignacio Machin ( .NET/ C# MVP )]

[...]
Just recently I updated it to 2005 but the code is the same, it works,
so why change it ?

For your own use?  I see no reason to change it.  For the purpose of  
providing suggestions to others?  I'm sure you can figure out what _I_  
think...  

yes, you are right, my bad
I do not know why but this is a frequent request, so all I do is a
search in google and I always find my old post with the old code.
Thank to you now this thread will be returned from now on.

To me, using a synchronized Queue makes a lot of sense if you're starting 
multiple threads with the same entry point but each with different input. 
But otherwise, it seems like overkill at best, and maybe a bit confusing  
and misleading at worst.  It's not _terrible_ in any case, but it does  
seem awkward and not very expressive.  

Pete

IMHO the solution with the queue seems clearer than the use of a
nested class to hold the parameter. You have to write much less code
and can be extended very easily for example when you need to change
the delegate at runtime.

 

[markgoldin]

Yes, each listener listen to a different port.
I am using 8 ports for now.

Here is my code, one for each port:
goJM4 = true;

// Buffer for reading data

Byte[] bytes = new Byte[1024];

TcpClient tcpc = DataAdapterLauncher.tcplJM4.AcceptTcpClient(); //accept
connection

String data;

while (goJM4)

{

data = "";

// Get a stream object for reading and writing

NetworkStream stream = tcpc.GetStream();

// Loop to receive all the data sent by the client.

int i = 0;

i = stream.Read(bytes, 0, bytes.Length);

while (i > 0)

{

i = stream.Read(bytes, 0, bytes.Length);

// Translate data bytes to a ASCII string.

data += System.Text.Encoding.ASCII.GetString(bytes, 0, i);

if (data.EndsWith("\r") || i == 0)

break;

}

// Process the data sent by the client.

if (data != "")

{

System.Console.WriteLine(data);

}

}


What is also happening that almost constantly I am not getting all data
being sent.
I am writing into SQL a record every time I am sending data and I dont have
anything missed but
in the Console the data shown is not everything I am sending. The one that
has been shown is complete
but not all messages are shown.
Any idea how to trouble shoot that?

Thanks

message

I am working on a program that creates a few TCP listenrs to accept data
from non .net processes.
Each listener listens in its own thread. I am using Thread.Sleep(1000) to
let other threads work.

you should only do this if each listener listen in a different post.

Here is my code:
tcpl = new TcpListener(ipe);
TcpClient tcpc = tcpl.AcceptTcpClient();
while (true)
{
// get data
while (true)
{
// if contidion met break;
}
Thread.Sleep(1000) ;

}

Do I really need Thread.Sleep(1000) ? What would happend if I dont use it
at
all?

why you use it in the first place??

Answer to this, are you using one port or multiple ports?
I can give you code in each case, it's as simple as using a SyncQueue.
and one thread per connection (no per listener)

 

[Markgoldin]

In your code sample:
protected void ListenerMethod()
{
TcpListener listener = new TcpListener(1212);

listener.Start();
while (true)
{
TcpClient client = listener.AcceptTcpClient();

new Thread(TheConnectionHandler).Start(client);
}
}
Why do you have TcpClient client = listener.AcceptTcpClient();
inside of the loop?
If to accept connections from differnt clients then does a client have to
disconnect every time after the data has been sent?

Thanks

[...]

I don't find the type "SyncQueue" in the MSDN documentation, so I don't
know what Ignacio is getting at. That said, I'll suggest that there are
really only two viable approaches to TCP network i/o in .NET:

There is none, but you have Queue.Synchronized method:
Returns a Queue wrapper that is synchronized (thread safe).

Seet his example:

All due respect and no offense intended, but... yuck!

You're using the queue just as a way of passing data from one thread to
another? What's wrong with using ParameterizedThreadStart? For example:

protected void ListenerMethod()
{
TcpListener listener = new TcpListener(1212);

listener.Start();
while (true)
{
TcpClient client = listener.AcceptTcpClient();

new Thread(TheConnectionHandler).Start(client);
}
}

protected void TheConnectionHandler(object objArg)
{
TcpClient client = (TcpClient)objArg;

//...
}

Notes:

-- I prefer to use the variable name "socket" only when it's actually
a Socket instance. TcpClient wraps a Socket, and can be used in similar
ways. But it's not really a Socket and thinking it is could lead to some
confusion.

-- The compiler can infer the correct type for the Thread constructor
and create the delegate for you, so you don't actually need to explicitly
specify "new ThreadStart(...)" or "new ParameterizedThreadStart(...)" when
creating the Thread instance.

A slight variation on the above theme would be to capture the "client"
variable in an anonymous method and avoid casting altogether:

protected void ListenerMethod()
{
TcpListener listener = new TcpListener(1212);

listener.Start();
while (true)
{
TcpClient client = listener.AcceptTcpClient();

new Thread(delegate { TheConnectionHandler(client) }).Start();
}
}

protected void TheConnectionHandler(TcpClient client)
{
//...
}

I still prefer the asynchronous methods. But the above are, I think, at
least more appropriate than creating a queue just for the purpose of
moving a single reference from one thread to another.

Just my two cents.

Pete

 

[User Groups]

Pete, thanks a lot for fixing my code. Really appreciate that.

I dont have any particular reason for 8 ports except that I have 8 clients
sending data to the .Net module.

These clients are scanner (data collection) servers. Users scan barcodes
which are going into different from .Net processes.

Then these processes send data to .Net module via sockets.

Creating separate ports for each scanning process should protect me from
concurrency problems when multiple users are scanning at the same time.

I have queuing mechanism on the data side too: I am trying not to send data
from multiple users at the same time, but for the sake of stability of the
program

I am also dedicating a port to each data stream.



protected void ListenerMethod()
{
TcpListener listener = new TcpListener(1212);


listener.Start();
while (true)
{
TcpClient client = listener.AcceptTcpClient();
new Thread(delegate { TheConnectionHandler(client); }).Start();
//new Thread(TheConnectionHandler).Start(client);
}
}

protected void TheConnectionHandler(object objArg)
{
TcpClient client = (TcpClient)objArg;

//...
}

I have a few questions about your code. How does exactly it work? Does it
create a new thread every time data has been sent?

Also where in the code do I extract data?

About having TcpClient client = listener.AcceptTcpClient(); inside of the
loop. I tried (before getting samples here) to have it inside, but

I wasn't eble to receive data more than once so I took it out of the loop.



Thanks again for all the help.

Yes, each listener listen to a different port.
I am using 8 ports for now.

Any particular reason why? One port should be sufficient, assuming the
rest of your code is designed correctly.

For sure, you've got at least one serious bug:

[...]
i = stream.Read(bytes, 0, bytes.Length);

while (i > 0)

{

i = stream.Read(bytes, 0, bytes.Length);

Whatever bytes you read from the stream in your first call to Read(), you
simply discard. I'm surprised that you get _any_ successful results with
this code. Maybe you have some bug elsewhere that somehow offsets this
bug.

And here's a line of code that could be bug if it was extrapolated to
other encodings:

// Translate data bytes to a ASCII string.

data += System.Text.Encoding.ASCII.GetString(bytes, 0, i);

I'd have to spend a little more time reviewing the Encoding class stuff,
but my recollection is that you need to get a single Encoding instance and
use it repeatedly when decoding strings from a byte stream. Otherwise, if
you receive partial data, it will get lost between calls to GetString().

Since ASCII is always only one byte per character, this isn't a problem
with ASCII per se. But if you were to change your design to transmit
encodings it would be.

But for now, the main thing you need to fix is to stop discarding the
first sequence of bytes you receive. As a minimal change to your code,
I'd suggest something like this:

i = stream.Read(bytes, 0, bytes.Length);

while (i > 0 && !data.EndsWith("\r"))
{
// Translate data bytes to a ASCII string.

data += System.Text.Encoding.ASCII.GetString(bytes, 0, i);

i = stream.Read(bytes, 0, bytes.Length);
}

You may also want to consider changing your code so that it uses a
StringBuilder to accumulate the string, rather than repeatedly
accumulating the string being read in. Whether it's really a problem
depends on how many times you're actually doing to concatenate and how
long the string will eventually get. But for large strings, StringBuilder
will be much more efficient.

Pete

 

[Markgoldin]

I am implemeting your code and I am getitng the following error:
Error 2 The call is ambiguous between the following methods or properties:
'System.Threading.Thread.Thread(System.Threading.ThreadStart)' and
'System.Threading.Thread.Thread(System.Threading.ParameterizedThreadStart)'
at this line:
new Thread(delegate { ConnectionHandler(tcpc); }).Start();

Can you help?

 

[Jon Skeet [C# MVP]]

I am implemeting your code and I am getitng the following error:
Error 2 The call is ambiguous between the following methods or properties:
'System.Threading.Thread.Thread(System.Threading.ThreadStart)' and
'System.Threading.Thread.Thread(System.Threading.ParameterizedThreadStart)'
at this line:
new Thread(delegate { ConnectionHandler(tcpc); }).Start();

Can you help?

That's using the form of anonymous method construction which doesn't
specify the parameters. You want to specify that you don't need any
parameters, so use:
delegate() { ConnectionHandler(tcpc); }

The () after delegate is the important bit.

Jon

 

[Markgoldin]

Here is my code after I applied everything I have gathered here:

public void Run()

{

go = true;

while (go)

{

TcpClient tcpc = DataAdapterLauncher.tcpl.AcceptTcpClient(); //accept
connection

new Thread(delegate() { ConnectionHandler(tcpc); }).Start();

}

}

protected void ConnectionHandler(object objArg)

{

TcpClient client = (TcpClient)objArg;

// Buffer for reading data

Byte[] bytes = new Byte[1024];

String data;

data = "";

// Get a stream object for reading and writing

NetworkStream stream = client.GetStream();

// Loop to receive all the data sent by the client.

int i = 0;

i = stream.Read(bytes, 0, bytes.Length);

while (i > 0 && !data.EndsWith("\r"))

{

// Translate data bytes to a ASCII string.

data += System.Text.Encoding.ASCII.GetString(bytes, 0, i);

i = stream.Read(bytes, 0, bytes.Length);

}

// Process the data sent by the client.

if (data != "")

{

System.Console.WriteLine(data);

IDictionary eventData = new Hashtable();

eventData["scan"] = data;

_listener.Update("floorupdate", eventData, false);

}

}

DataAdapterLauncher.tcpl starts in another class:

IPEndPoint ipe = new IPEndPoint(IPAddress.Parse("0.0.0.0"), 63335);//listen
on all local addresses
tcpl = new TcpListener(ipe);

tcpl.Start();

As I was saying it only accepts data once.

I am checking my client side and I see that data is getitng sent but
while (go)

{

TcpClient tcpc = DataAdapterLauncher.tcpl.AcceptTcpClient(); //accept
connection

new Thread(delegate() { ConnectionHandler(tcpc); }).Start();

}

with
System.Console.WriteLine(1); in it

shows 1 only once.

Any idea?

 

[Markgoldin]

I am sorry for the code hardly readable.
I did not publish my client side code because it's not a .Net code.
My client side is using a Winsock ActiveX control.
Here is an issue I am having if I am accepting connection inside of the
loop.
The only way it would work FOR ME is when a client closes connection every
time after data is being sent.
That's fine from a coding stand point of view but not from a performance
one.
In order to send data with the following connection closing I have to delay
closing for something like .05 seconds or maybe even more.
While that does not look like too long but in Production it is noticable and
not desuried but the users.
Is there any way (even in theory) to keep a permanent connection between a
client and a server?
That way I won't need to close connection.

Thanks for all your help.

[...]
I am checking my client side and I see that data is getitng sent but
while (go)

{

TcpClient tcpc = DataAdapterLauncher.tcpl.AcceptTcpClient(); //accept
connection

new Thread(delegate() { ConnectionHandler(tcpc); }).Start();

}

with
System.Console.WriteLine(1); in it

shows 1 only once.

Any idea?

Well, how many times do you try to connect to the listener?

AcceptTcpClient() is only going to return once for every time some remote
endpoint tries to actually connect.

You didn't post the client code, but given that AcceptTcpClient() only
appears to be returning a new connection once, the client is probably only
trying to connect once.

For what it's worth, your code samples so far have been very hard to read,
because they lack any indentation. For future posts, it would be very
helpful if you would make sure that your code has some indentation (three
or four spaces per block level would be nice). And ideally, you would
post a concise-but-complete sample...that is, your code should be
self-contained, compilable without error and reliably demonstrate the
issue. For networking issues, that means you need to post code for both
ends, client and server.

Pete