Select() Bad file descriptor

Hello everyone,
I am trying to use a library found on the Internet for communication with network sockets, implementing a client-server communication.
In the client after the connect () call runs a loop that executes 10 iterations, each iteration sends a message to the server and waits for a response.
The server with the select() waits for connections, once received the message from the client sends a response.
At the first iteration is all ok, at the second the server gives me a bad file descriptor error when it executes the select again, I can not deal with it, can anyone help? Thanks!
Place the server code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
#include <iostream>
#include "usock.h"
#include "usock_exception.h"

#include <string.h>


using namespace std;
using namespace usock;



int main() {
printf("Executing program prova_select\n");
int val=1;
char send_buffer[BUFRECV_SIZE];
char recv_buffer[BUFRECV_SIZE];

int max_sd, new_sd;
int clients[10];
int client_index=-1;

fd_set readset,tempset;

struct sockaddr_in addr;
socklen_t len = sizeof(struct sockaddr);


try {
ServerSocket ss(9999);

printf("Server: listening on port %d\n", ss.localPort());

try{
ss.setSockOpt(SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
}
catch (exception e) {
printf("Reuse address exception");
exit(-1);
}

max_sd = ss.getSd();

for (int ind = 0; ind < 10; ind++) {
clients[ind]=-1;
}

FD_ZERO(&readset); //Resetto l'insieme dei descrittori per la lettura
FD_SET(ss.getSd(), &readset); //Setto il bit corrispondente al socket in ascolto nell'insieme dei descrittori per la lettura

while(1) {

tempset = readset;
if (select(max_sd+1, &tempset, NULL, NULL, NULL) < 0) {
perror("select()");
throw SocketException("Server: select error");
}

if (FD_ISSET(ss.getSd(), &tempset)) {
try {
int new_sd=accept(ss.getSd(), (struct sockaddr*)&addr, &len);
printf("Server: Accepted new connection on socket %d\n", new_sd);


for (int i = 0; i < ss.getMaxconn(); i++) {
if (-1 == clients[i]) {
clients[i]=new_sd;

//aggiorna massimo indice in array di connessioni aperte
if (i > client_index) {
client_index=i;

}
FD_SET(new_sd, &readset);

//Aggiorno indice del massimo descrittore per la select
if (new_sd > max_sd) {
max_sd=new_sd;
}
break;
}
}
}
catch (exception e) {
printf("Server: accept() failed\n");
}
}

else {
for (int i=0; i <= 10; i++) {
int temp_sd=clients[i];

if (-1 == temp_sd) {
printf("Server: Socket %d already closed\n", temp_sd);
continue;
}

//Controllo se sono stati ricevuti dati sul socket corrente
if ( FD_ISSET(temp_sd, &tempset) ) {


try {
Socket temp_sock(temp_sd);
//Controllo il numero dei bytes ricevuti. Se è pari a zero la connessione lato client è stata chiusa. Rimuovo il descrittore dall'array delle connessioni aperte
if ( temp_sock.recv(recv_buffer, sizeof(recv_buffer)) == 0 ) {

printf("Server: Socket %d closed.\n", temp_sd);
temp_sock.close();
FD_CLR(temp_sd, &readset);
clients[i]=-1;
}

else {

printf("Server: Received data %s from socket %d\n", recv_buffer, temp_sd);
strcpy(send_buffer, recv_buffer);

try {

temp_sock.send(send_buffer, sizeof(send_buffer));

}
catch (exception e) {
printf("Send failed");

}

}

}
catch (exception e) {
printf("Receive failed\n");
}
}
}
}
}

ss.close();
}
catch (exception e) {
exit(-1);
}
}
Last edited on
Please edit your post and put the source inside code tags. It will make it a lot more legible and folks here will be more likely to look at it.
I take it this was written by a Java developer...all those annoying/distracting exceptions and those braces ...

BTW, catching exception classes by value like that is bad form.

The whole point of exceptions is to remove the error handling from the code, not embed it in the most verbose way possible. The algorithm is almost invisible.

If ss.getMaxconn() is larger than 10, you'll have a problem.

Variables aren't declared where they're used, leading you to declare two new_sd variables and more importantly, not initialising clients.
1
2
3
4
for (int ind = 0; ind < 10; ind++)
{
	clients[i]=-1;	// shoudl be clients[ind]
}


The uninitialised clients is causing your problem.
Last edited on
I realize that with all these exceptions, the code is hard to read ... How do you suggest to catch exceptions?
You're right, it should be clients [ind], it was a stupid mistake on my part in posting the code.

maxconn is a variable of the ServerSocket class and has default value 10.

For what I understood the problem is that I create the variable temp_sock to handle communications with the client. In fact, if instead of temp_sock.recv() and temp_sock.send() I use the system calls recv() and send() the problem does not occur.
But I need to program with objects, how can I avoid this error?
Last edited on
First, don't call you mistakes stupid. The machine makes fools of us all. These Von Neumann languages are prone to such errors.

If you declare variables where they're used, you're less likely to use the wrong varaible, and uninitialised variable, not consider release and so on.

If getMaxconn() returns a constant, you should declare your array to be this size. Of course you can't declare an array with the a size determined at runtime, so you'd probably use a vector instead. The point is, if getMaxconn() is the upper limit, the code should reflect that somehow. I'ts worth noting that the actuall socket library doesn't impose such limits.

temp_sock.recv() should be ok and should be equivalend to ::recv(temp_sock, . If they're different, it may indicate a problem in the socket library's implementation.

Most of your exceptions just print an error message and terminate. For example, there's a catch block around accept, but nothing in there can throw an exception. So, remove the unnecessary try/catch stuff and just have a single catch at the bottom. The code is littered with calls to perror, throwing different types of errors and so on. Just throw std::runtime_error with a message, or something simple like that.
Last edited on
Solved! I didn't know that when I exit from the context is automatically closed the destructor method, and in the destructor I call the close() system call. Ok, I am a bit ignorant, thsnks kbw for your suggestions
Topic archived. No new replies allowed.