for practicing wanted create client , server applications simulate lobby.
therefore, in server-application accept incoming connections, create clientinfo object containing tcpclient object, usernames, id, etc. , methods sending , receiving data, , store clientinfo object in list in lobby class. when user chatting, message being sent server , broadcasted available clients.
the problem have is: first client connects. broadcasts go defaultuser1. second client connects. broadcasts go defaultuser2 + defaultuser2.
as can see, first client not receiving data anymore, nor can server receive data him. somehow data in list must corrupted. here relevant bit of code:
accepting incoming conenctions , creating clientinfo object , storing lobby:
while (mworking) { tcpclient client = mlistener.accepttcpclient(); mnumberofclients++; console.writeline("new tcp-connection client: " + client.client.localendpoint.tostring()); clientinfo newinfo = new clientinfo(client, mnumberofclients); mlobby.addclient(newinfo); } the clientinfo constructor:
public clientinfo(tcpclient client, int clientnumber) { mclient = client; mclientnumber = clientnumber; musername = "defaultuser" + mclientnumber.tostring(); mstream = client.getstream(); mencoding = new asciiencoding(); } the sending method in clientinfo:
public void send(string message) { mcurrentmessage = message; thread sendthread = new thread(this.writetask); sendthread.start(); } private void writetask() { byte[] data = mencoding.getbytes(mcurrentmessage); byte[] sizeinfo = new byte[4]; sizeinfo[0] = (byte)data.length; sizeinfo[1] = (byte)(data.length >> 8); sizeinfo[2] = (byte)(data.length >> 16); sizeinfo[3] = (byte)(data.length >> 24); mstream.write(sizeinfo, 0, sizeinfo.length); mstream.write(data, 0, data.length); } relevant code in lobby class:
private static list<clientinfo> mclients; private static processdel mprocessdel; public lobby(processdel del) { mprocessdel = del; mclients = new list<clientinfo>(); } public void addclient(clientinfo client) { mclients.add(client); client.listen(mprocessdel); broadcast("ujoin§" + client.username + "$"); } public void broadcast(string message) { (int = 0; < mclients.count; i++) { console.writeline("broadcasting " + mclients[i].username); mclients[i].send(message); } } i tried broadcasting foreach, same result. processdel delegate method need processing received data. receiving handled seperate thread each client.
as guess, seems misunderstood static means in c#.
static means method or field part of type, rather instance of type. if of fields static, don't have instance data, , state shared across instances of class - second client overwrites data associated first client well. solution simple - remove statics, , should fine.
other that, code has thread-safety issues. types in .net not thread-safe default, , need add appropriate locking make sure consistency maintained. more of topic codereview, perhaps, i'll note first things come mind:
sendstarts new thread send message. however, means if it's called twice in succession under right conditions, can corrupt tcp stream - example, first thread might write length data, second writes length data before first writes actual data , you're in trouble. it's possible you'd send second message twice, since you're passing text send through field.list<t>isn't thread-safe. means can safely use single thread - it's not entirely clear code, seems might have trouble that. usingconcurrentdictionary<ipendpoint, clientinfo>might better idea, depends on you're doing.
you explore alternative options, using asynchronous i/o instead of spamming threads, that's bit more advanced option (mind you, multi-threading worse :)). regardless, start thread-safety http://www.albahari.com/threading/ it's long, multi-threading is complex , dangerous topic, , tend produce errors hard find , reproduce, while running in debugger.
Comments
Post a Comment