From 18f5ff7be0a8f8a1f02156e8ae3f08f824166048 Mon Sep 17 00:00:00 2001 From: Christian Pointner Date: Tue, 26 May 2015 03:43:04 +0200 Subject: make fd states more explicit advanced close scenario works now for well behaved server/clients --- src/clients.c | 158 +++++++++++++++++++++++++++++++++------------------------- src/clients.h | 2 +- 2 files changed, 90 insertions(+), 70 deletions(-) diff --git a/src/clients.c b/src/clients.c index dfd00cc..68c6fb6 100644 --- a/src/clients.c +++ b/src/clients.c @@ -226,6 +226,18 @@ void clients_print(clients_t* list) } } +static char* client_fd_state_to_string(client_fd_state_t s) +{ + switch(s) { + case ESTABLISHING: return "establishing"; + case ESTABLISHED: return "established"; + case RCV_STOPPED: return "receiving stopped"; + case FIN_PENDING: return "FIN pending"; + case FIN_LINGER: return "FIN linger"; + case CLOSE_PENDING: return "CLOSE pending"; + } +} + void clients_read_fds(clients_t* list, fd_set* set, int* max_fd) { if(!list) @@ -235,19 +247,20 @@ void clients_read_fds(clients_t* list, fd_set* set, int* max_fd) while(tmp) { client_t* c = (client_t*)tmp->data_; if(c && (c->state_ == CONNECTED || c->state_ == CLOSING)) { - if(c->fd_state_[0] != CLOSE_PENDING && c->fd_state_[0] != CLOSE_LINGER - && c->write_buf_offset_[1] < c->write_buf_[1].length_) { - FD_SET(c->fd_[0], set); - *max_fd = *max_fd > c->fd_[0] ? *max_fd : c->fd_[0]; - - log_printf(DEBUG, "adding %d for READ", c->fd_[0]); - } - if(c->fd_state_[1] != CLOSE_PENDING && c->fd_state_[1] != CLOSE_LINGER - && c->write_buf_offset_[0] < c->write_buf_[0].length_) { - FD_SET(c->fd_[1], set); - *max_fd = *max_fd > c->fd_[1] ? *max_fd : c->fd_[1]; - - log_printf(DEBUG, "adding %d for READ", c->fd_[1]); + int i; + for(i=0; i<2; ++i) { + if(c->write_buf_offset_[i^1] < c->write_buf_[i^1].length_) { + switch(c->fd_state_[i]) { + case ESTABLISHED: + case FIN_PENDING: + case FIN_LINGER: { + FD_SET(c->fd_[i], set); + *max_fd = *max_fd > c->fd_[i] ? *max_fd : c->fd_[i]; + log_printf(DEBUG, "adding %d for READ", c->fd_[i]); + } + default: /* not adding fd */; + } + } } } tmp = tmp->next_; @@ -263,19 +276,21 @@ void clients_write_fds(clients_t* list, fd_set* set, int* max_fd) while(tmp) { client_t* c = (client_t*)tmp->data_; if(c && (c->state_ == CONNECTED || c->state_ == CLOSING)) { - if((c->fd_state_[0] == ESTABLISHED || c->fd_state_[0] == CLOSE_PENDING || c->fd_state_[0] == FIN_PENDING) - && c->write_buf_offset_[0]) { - FD_SET(c->fd_[0], set); - *max_fd = *max_fd > c->fd_[0] ? *max_fd : c->fd_[0]; - - log_printf(DEBUG, "adding %d for WRITE", c->fd_[0]); - } - if((c->fd_state_[1] == ESTABLISHED || c->fd_state_[1] == CLOSE_PENDING || c->fd_state_[1] == FIN_PENDING) - && c->write_buf_offset_[1]) { - FD_SET(c->fd_[1], set); - *max_fd = *max_fd > c->fd_[1] ? *max_fd : c->fd_[1]; - - log_printf(DEBUG, "adding %d for WRITE", c->fd_[1]); + int i; + for(i=0; i<2; ++i) { + if(c->write_buf_offset_[i]) { + switch(c->fd_state_[i]) { + case ESTABLISHED: + case RCV_STOPPED: + case FIN_PENDING: + case CLOSE_PENDING: { + FD_SET(c->fd_[i], set); + *max_fd = *max_fd > c->fd_[i] ? *max_fd : c->fd_[i]; + log_printf(DEBUG, "adding %d for WRITE", c->fd_[i]); + } + default: /* not adding fd */; + } + } } } else if(c && c->state_ == CONNECTING) { FD_SET(c->fd_[1], set); @@ -287,29 +302,25 @@ void clients_write_fds(clients_t* list, fd_set* set, int* max_fd) } } -static char* client_fd_state_to_string(client_fd_state_t s) -{ - switch(s) { - case ESTABLISHING: return "establishing"; - case ESTABLISHED: return "established"; - case FIN_PENDING: return "FIN pending"; - case FIN_LINGER: return "FIN linger"; - case CLOSE_PENDING: return "CLOSE pending"; - case CLOSE_LINGER: return "CLOSE linger"; - } -} - -static int client_handle_close(client_t* c, int in, int out) +static int client_handle_recv_null(client_t* c, int in, int out) { log_printf(DEBUG, "client %d: recv(%d) returned 0, %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], c->fd_[in], c->fd_[0], c->write_buf_offset_[0], client_fd_state_to_string(c->fd_state_[0]), c->fd_[1], c->write_buf_offset_[1], client_fd_state_to_string(c->fd_state_[1])); c->state_ = CLOSING; - c->fd_state_[in] = (c->write_buf_offset_[in]) ? CLOSE_PENDING : CLOSE_LINGER; + + switch(c->fd_state_[in]) { + case ESTABLISHING: log_printf(ERROR, "client %d: socket %d is in the ESTABLISHING state???", c->fd_[0], c->fd_[in]); return 1; // assert() this?? + case ESTABLISHED: c->fd_state_[in] = RCV_STOPPED; break; + case RCV_STOPPED: log_printf(ERROR, "client %d: socket %d is already in RCV_STOPPED state???", c->fd_[0], c->fd_[in]); return 1; // assert() this?? + case FIN_PENDING: /* do nothing */; break; + case FIN_LINGER: /* do nothing */; break; + case CLOSE_PENDING: log_printf(ERROR, "client %d: socket %d is already in CLOSE_PENDING state???", c->fd_[0], c->fd_[in]); break; + } switch(c->fd_state_[out]) { - case ESTABLISHING: return 1; // this will never be reached - remove client for now > assert() this in future?? + case ESTABLISHING: log_printf(ERROR, "client %d: socket %d is in the ESTABLISHING state???", c->fd_[0], c->fd_[out]); return 1; // assert() this?? case ESTABLISHED: { if(c->write_buf_offset_[out]) { c->fd_state_[out] = FIN_PENDING; @@ -319,13 +330,43 @@ static int client_handle_close(client_t* c, int in, int out) } break; } - case FIN_PENDING: log_printf(ERROR, "client %d: socket %d is already in FIN_PENDING state???", c->fd_[0], c->fd_[out]); break; - case FIN_LINGER: log_printf(ERROR, "client %d: socket %d is already in FIN_LINGER state???", c->fd_[0], c->fd_[out]); break; + case RCV_STOPPED: { + if(c->write_buf_offset_[out]) { + c->fd_state_[out] = CLOSE_PENDING; + } else { + log_printf(INFO, "client %d: both connections are finished now - removing it", c->fd_[0]); + return 1; + } + break; + } + case FIN_PENDING: log_printf(ERROR, "client %d: socket %d is already in FIN_PENDING state???", c->fd_[0], c->fd_[out]); break; // assert() this?? + case FIN_LINGER: log_printf(ERROR, "client %d: socket %d is already in FIN_LINGER state???", c->fd_[0], c->fd_[out]); break; // assert() this?? case CLOSE_PENDING: log_printf(DEBUG, "client %d: socket %d has data pending - removal postponed", c->fd_[0], c->fd_[out]); break; - case CLOSE_LINGER: log_printf(INFO, "client %d: both connections are closed now - removing it", c->fd_[0]); return 1; } - log_printf(DEBUG, "client %d is now %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], + log_printf(DEBUG, "client %d state is now %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], + c->fd_[0], c->write_buf_offset_[0], client_fd_state_to_string(c->fd_state_[0]), + c->fd_[1], c->write_buf_offset_[1], client_fd_state_to_string(c->fd_state_[1])); + + return 0; +} + +static int client_handle_buffer_flushed(client_t* c, int i) +{ + log_printf(DEBUG, "client %d: cleared write buffer[%d], %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], c->fd_[i], + c->fd_[0], c->write_buf_offset_[0], client_fd_state_to_string(c->fd_state_[0]), + c->fd_[1], c->write_buf_offset_[1], client_fd_state_to_string(c->fd_state_[1])); + + switch(c->fd_state_[i]) { + case ESTABLISHING: log_printf(ERROR, "client %d: socket %d is in the ESTABLISHING state???", c->fd_[0], c->fd_[i]); return 1; // assert() this?? + case ESTABLISHED: + case RCV_STOPPED: /*nothing here*/; break; + case FIN_PENDING: c->fd_state_[i] = FIN_LINGER; shutdown(c->fd_[i], SHUT_WR); break; + case FIN_LINGER: log_printf(ERROR, "client %d: socket %d is already in FIN_LINGER state???", c->fd_[0], c->fd_[i]); return 1; // assert() this?? + case CLOSE_PENDING: log_printf(INFO, "client %d: both connections are finished now - removing it", c->fd_[0]); return 1; + } + + log_printf(DEBUG, "client %d state is now %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], c->fd_[0], c->write_buf_offset_[0], client_fd_state_to_string(c->fd_state_[0]), c->fd_[1], c->write_buf_offset_[1], client_fd_state_to_string(c->fd_state_[1])); @@ -360,7 +401,7 @@ int clients_read(clients_t* list, fd_set* set) break; } else if(!len) { - if(client_handle_close(c, in, out)) { + if(client_handle_recv_null(c, in, out)) { slist_remove(&(list->list_), c); break; } @@ -374,26 +415,6 @@ int clients_read(clients_t* list, fd_set* set) return 0; } -static int client_handle_pending(client_t* c, int i) -{ - log_printf(DEBUG, "client %d: cleared write buffer[%d], %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], c->fd_[i], - c->fd_[0], c->write_buf_offset_[0], client_fd_state_to_string(c->fd_state_[0]), - c->fd_[1], c->write_buf_offset_[1], client_fd_state_to_string(c->fd_state_[1])); - - if(c->fd_state_[i] == CLOSE_PENDING) { - c->fd_state_[i] = CLOSE_LINGER; - } else { - c->fd_state_[i] = FIN_LINGER; - } - shutdown(c->fd_[i], SHUT_WR); - - log_printf(DEBUG, "client %d is now %d: bytes=%d state=%s, %d: bytes=%d state=%s", c->fd_[0], c->fd_[i], - c->fd_[0], c->write_buf_offset_[0], client_fd_state_to_string(c->fd_state_[0]), - c->fd_[1], c->write_buf_offset_[1], client_fd_state_to_string(c->fd_state_[1])); - - return 0; -} - int clients_write(clients_t* list, fd_set* set) { if(!list) @@ -423,9 +444,8 @@ int clients_write(clients_t* list, fd_set* set) } else { c->write_buf_offset_[i] = 0; - if(c->fd_state_[i] == CLOSE_PENDING || c->fd_state_[i] == FIN_PENDING) - if(client_handle_pending(c, i)) - slist_remove(&(list->list_), c); + if(client_handle_buffer_flushed(c, i)) + slist_remove(&(list->list_), c); } } } diff --git a/src/clients.h b/src/clients.h index ce0f371..df2c455 100644 --- a/src/clients.h +++ b/src/clients.h @@ -37,7 +37,7 @@ enum client_state_enum { CONNECTING, CONNECTED, CLOSING }; typedef enum client_state_enum client_state_t; -enum client_fd_state_enum { ESTABLISHING, ESTABLISHED, FIN_PENDING, FIN_LINGER, CLOSE_PENDING, CLOSE_LINGER }; +enum client_fd_state_enum { ESTABLISHING, ESTABLISHED, RCV_STOPPED, FIN_PENDING, FIN_LINGER, CLOSE_PENDING }; typedef enum client_fd_state_enum client_fd_state_t; typedef struct { -- cgit v1.2.3