SFML community forums
Help => Network => Topic started by: The Inzuki on May 12, 2011, 03:32:16 am
-
Hello. I've been working with sockets and threading for a while and came across an error that has been bugging me for a few hours now. The problem is that I'm getting access violations because the client isn't receiving anything in the separate thread I've created for receiving. After a few more tries, it's not receiving because apparently the client fails to bind to any port. In the main thread, it's perfectly bound to 0 (to prevent issues if the server and client are on the same computer), but when I bind it to 0 or any other port in the separate thread, it errors.
Any help would be appreciated, thanks!
-
Well I just tried SetBlocking(false) on the client's socket, and included all the receiving code in an if statement in the main loop. Now the client's console gets spammed saying that it cannot bind the port.
-
You should show us a complete and minimal example that reproduces the problem.
-
Well before the while loop that sends and receives, I have the client bound.
if(!Client.Bind(LPort)) // LPort being 0
return 1;
I also have declared an sf::Packet called Ser for receiving and Name that's used for sending.
And here's the actual receiving part, the code that for some reason says that port is unbound:
if(Client.Receive(Ser, Server, Port) == sf::Socket::Done){
if(Ser.GetData()[0] == 1){
sprintf(buf, "%s", Ser.GetData());
for(int i = 0; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
slot = atoi(buf);
std::cout << "<Given slot [" << slot << "]>" << std::endl;
}else if(Ser.GetData()[0] == 2){
sprintf(buf, "%s", Ser.GetData());
for(int i = 0; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
int connected = atoi(strtok(buf, ":"));
for(int i = 0; i < connected; i++) {
int tId = atoi(strtok(NULL, ","));
u[tId].name[255] = atoi(strtok(NULL, ";"));
}
}
}
I hope this was minimal to you; if you need to know what the code does, it checks for when it receives data from the server, and then it checks for what character is in front of the packet. Depending on what it is, it either connects the client (which is the first check) or sends data of all connected players (second check).
-
for(int i = 0; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
for(int i = 0; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
In both examples you access buf[-1] which is illegal.
-
Well oddly enough I've never had a problem with that, and it errors on the line with:
if(Ser.GetData()[0] == 1){
...since it can't read any data since it didn't read any data because of the client not being bound, when it clearly is.
I fixed it though, made it start at 1 instead of 0.
-
I'm not sure if this forum is strict on the use of "bumping a thread," so I hope you don't mind if I do.
-
There's no rule against bumps, if you feel like you need to bump your thread to get more help I don't mind ;)
However your last message was only three days old and was still in the three first messages of the network forum, so I don't know if bumping will help.
Like I said in my first answer, without a minimal and complete code (one that we can test and that focuses on the problem -- not pieces of your original program) nobody will be able to help you efficiently.
-
Oh, that's what you meant by minimal code.
The code isn't that large, so hopefully the whole code will be minimal.
Client:
#include <SFML/Audio.hpp>
#include <SFML/Graphics.hpp>
#include <SFML/Network.hpp>
#include <SFML/System.hpp>
#include <SFML/Window.hpp>
#include <iostream>
#include <stdio.h>
#include <string.h>
#include <process.h>
struct User {
bool exists;
char name[255], ip[16];
};
short slot = 0;
int main(int argc, char **argv){
char buf[255];
bool *close;
close = false;
User *u = new User[4];
sf::SocketUDP Client;
sf::IPAddress Server;
Client.SetBlocking(false);
char name[255], ip[255];
printf("Enter username: "); scanf("%s", name);
printf("Enter IP (enter lh for localhost): "); scanf("%s", ip);
if(strcmp("lh", ip) == 0)
Server = sf::IPAddress::LocalHost;
else
Server = sf::IPAddress(ip);
if(!Server.IsValid())
return 1;
if(!Client.Bind(0))
return 1;
sf::Packet Ser;
sf::Packet Name;
sprintf(buf, "%c%s", 1, name);
Name.Append(buf, sizeof(buf));
Client.Send(Name, Server, 1357);
std::cout << "Successfully connected!" << std::endl;
sf::RenderWindow App(sf::VideoMode(640, 480), "SFML Sockets");
App.EnableKeyRepeat(false);
App.SetFramerateLimit(60.0);
sf::Event e;
while(App.IsOpened()){
Name.Clear();
App.Clear();
if(Client.Receive(Ser, Server, 1357) == sf::Socket::Done){
if(Ser.GetData()[0] == 1){
sprintf(buf, "%s", Ser.GetData());
for(int i = 1; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
slot = atoi(buf);
std::cout << "<Given slot [" << slot << "]>" << std::endl;
}else if(Ser.GetData()[0] == 2){
sprintf(buf, "%s", Ser.GetData());
for(int i = 1; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
int connected = atoi(strtok(buf, ":"));
for(int i = 0; i < connected; i++) {
int tId = atoi(strtok(NULL, ","));
u[tId].name[255] = atoi(strtok(NULL, ";"));
}
}
}
if(App.GetEvent(e)){}
App.Display();
}
Client.Close();
return 0;
}
Server:
#include <SFML/Network.hpp>
#include <iostream>
#include <stdio.h>
#include <string.h>
#include <process.h>
struct User {
bool exists;
char name[255], ip[16];
};
short giveSlot(User *u, int max);
int main(int argc, char **argv){
unsigned short Port = 1357;
short max = 4, connected = 0;
User *u = new User[max];
sf::SocketUDP Server;
sf::IPAddress Sender;
for(int i = 1; i < max; i++)
u[i].exists = false;
if(!Server.Bind(Port))
return 1;
char buf[255], bufs[255];
sf::Packet Name;
sf::Packet Sen;
while(true){
short slot = giveSlot(u, max); // gives the next open slot the user
Server.Receive(Name, Sender, Port);
if(Name.GetData()[0] == 1){
sprintf(buf, "%s", Name.GetData());
for(int i = 1; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
std::cout << buf << " (" << Sender << ") has connected! <Given slot [" << slot << "]>" << std::endl;
u[slot].exists = true;
u[slot].name[255] = buf[255];
Sen.Clear();
sprintf(bufs, "%c%c", 1, u[slot]);
Sen.Append(bufs, sizeof(bufs));
Server.Send(Sen, Sender, Port);
for(int i = 1; i <= max; i++){
if(u[i].exists == true){
if(i != slot){
Sen.Clear();
sprintf(bufs, "%c%i:%i,%s;", 2, connected, i, u[i].name);
Sen.Append(bufs, sizeof(bufs));
Server.Send(Sen, Sender, Port);
}
}
}
}else if(Name.GetData()[0] == 2){
sprintf(buf, "%s", Name.GetData());
for(int i = 0; i < sizeof(buf); i++){
buf[i - 1] = buf[i];
}
std::cout << buf << " (" << Sender << ") has disconnected!" << std::endl;
//u[slot].exists = false;
//memset(u[slot].name, 0, sizeof(u[slot].name));
}
}
Server.Close();
return 0;
}
EDIT: Oh dear, now that I look at it, it does seem a bit long..
-
EDIT: Oh dear, now that I look at it, it does seem a bit long..
Yep, I'm not sure that we need all the stuff that you included in it, to solve your problem.
-
Alright, I'll edit the post and take out the unnecessary parts.
-
I couldn't really take much out without messing up most of it, but I could try removing some variables too.
-
You should first know exactly what's wrong. You're talking about "access violation", "clients fails to bind", "not receiving anything", but to me these are three different problems. You must find out which one triggers all the others, and only focus on it. Then you also talk about threads, but I don't see any in your minimal code.
-
I tried threading but removed it so I could try the SetBlocking() function for sockets. This didn't fix the problem of course, and the access violation is occuring because of the if statement checking if the receiving is equal to sf::Socket::Done. I guess it can't check since it can't receive anything due to the client not being bound to a port. So they're all pretty much linked together.
-
This didn't fix the problem of course, and the access violation is occuring because of the if statement checking if the receiving is equal to sf::Socket::Done. I guess it can't check since it can't receive anything due to the client not being bound to a port.
Hum...
1. how do you know that the access violation occurs because of the if statement?
2. how do you know that the client is not bound?
It seems that you're jumping to (incorrect) conclusions. If Bind failed, it would have returned an error. And even if it happens, Receive doesn't crash. SFML is not that stupid ;)
So now let's restart, please tell us what happens, without trying to figure out the cause. Use the debugger, you'll know where it crashes.
There are also many things that are not safe in your code. Get rid of them (they seem to be really useless), or at least make them safer (use std::string, std::ostringstream).
-
Oops, totally forgot that the access violation doesn't happen anymore. The problem now is that the client's console is spammed with "Failed to receive data : the UDP socket first needs to be bound to a port".
-
Ok. But please can you remove all this useless stuff?? We just need a simple Send/Receive to test the bug, if Receive fails we don't need the whole block that follows the call (since it's not executed, and not relevant to the problem anyway). And I don't think we need a window...
-
I think I might have an explanation of what is wrong. It appears that the client is trying to receive on a port that it hasn't bound. Notice how the code below is trying to receive on port 1357, but earlier it was bound to port 0 (which amounts to an ephemeral, or random, port assigned by the OS).
Client:
...
if(!Client.Bind(0))
return 1;
...
Client.Send(Name, Server, 1357);
...
if(Client.Receive(Ser, Server, 1357) == sf::Socket::Done){
...
Client.Close();
Normally, when creating a network application your server has a fixed port and the client uses an ephemeral port like you have already shown above. When the client wants to send data to the server is does so to the fixed port provided above. But when the client wants to receive data from the server it must do so on the ephemeral port assigned by the OS. I'm not sure how SFML handles returning ephemeral ports, there is probably some Client.GetPort() call that can be made to determine the ephemeral port assigned. If that is the case, you should be able to do a Client.Receive using this port and prevent a socket not bound error. Also, you must be careful to keep track of each clients ephemeral port as they connect on the server side of your code. The connection packet received can be used to obtain the ephemeral port being used by the client so that all future server messages sent from the server to the clients will use the correct ephemeral port.
Hopefully this information was useful (and accurate), please let us know how it works out.
-
I think you spotted the error but it's not what you thought.
This line:
Client.Receive(Ser, Server, 1357)
... doesn't mean that the client tries to receive on port 1357; UDP sockets always receive on the port they are bound to.
In fact this line shouldn't compile, the third argument of SocketUDP::Receive is a reference to an integer to fill with the remote port where the message came from. So there's no way it can compile with a literal integer as the third argument.
-
Thanks again for the clarification, I knew my answer was probably not quite right, I didn't have time to check the docs, perhaps the code that was pasted was slightly modified but not compiled before being posted.