Skip to content

Commit 098288a

Browse files
committed
Revert "TCP: Fix race condition in "proto_tcp.tcp_parallel_handling""
This reverts commit d279257.
1 parent 67d7400 commit 098288a

File tree

5 files changed

+13
-38
lines changed

5 files changed

+13
-38
lines changed

modules/proto_tls/proto_tls.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ static int proto_tls_send(struct socket_info* send_sock,
629629

630630
static int tls_read_req(struct tcp_connection* con, int* bytes_read)
631631
{
632-
int ret, rc = 0;
632+
int ret;
633633
int bytes;
634634
int total_bytes;
635635
struct tcp_req* req;
@@ -733,7 +733,7 @@ static int tls_read_req(struct tcp_connection* con, int* bytes_read)
733733
int max_chunks = tcp_attr_isset(con, TCP_ATTR_MAX_MSG_CHUNKS) ?
734734
con->profile.attrs[TCP_ATTR_MAX_MSG_CHUNKS] : tls_max_msg_chunks;
735735

736-
switch ((rc = tcp_handle_req(req, con, max_chunks, 0))) {
736+
switch (tcp_handle_req(req, con, max_chunks, 0) ) {
737737
case 1:
738738
goto again;
739739
case -1:
@@ -743,9 +743,8 @@ static int tls_read_req(struct tcp_connection* con, int* bytes_read)
743743
LM_DBG("tls_read_req end\n");
744744
done:
745745
if (bytes_read) *bytes_read=total_bytes;
746-
747-
return rc == 2 ? 1 /* connection is already released! */
748-
/* 0,1? */: 0; /* connection will be released */
746+
/* connection will be released */
747+
return 0;
749748
error:
750749
/* connection will be released as ERROR */
751750
return -1;

net/api_proto_net.h

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,25 +34,6 @@
3434

3535

3636
typedef int (*proto_net_write_f)(void *src, int fd);
37-
38-
/**
39-
* Read a complete SIP message from the network, including its body/payload and
40-
* also fully process it, through receive_msg() and opensips.cfg routing logic.
41-
*
42-
* The function must include sufficient parsing logic in order to overcome
43-
* the streaming protocol's inherent limitation of not having the data "split"
44-
* into individual application-layer messages. For example, implementors may
45-
* want to block-READ until "Content-Length: " to obtain payload size, then
46-
* continue READ'ing until EOH, then a final READ for the full body.
47-
*
48-
* Possible return values:
49-
* -1 :: error during reading, the @src connection is still valid and must be
50-
* released by caller
51-
* 0 :: a complete request was read and processed; check @src->state for
52-
* S_CONN_EOF and release @src connection if condition is true
53-
* 1 :: a complete request was read and processed; the @src ptr is *invalid*
54-
* as the connection has already been released, to achieve parallelism
55-
*/
5637
typedef int (*proto_net_read_f)(void *src, int *len);
5738
typedef int (*proto_net_conn_init_f)(struct tcp_connection *c);
5839
typedef void (*proto_net_conn_clean_f)(struct tcp_connection *c);

net/net_tcp_proc.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,6 @@ inline static int handle_io(struct fd_map* fm, int idx,int event_type)
344344
"handle read, err, resp: %d, att: %d",
345345
resp, con->msg_attempts);
346346
tcpconn_release_error(con, 0, "Read error");
347-
} else if (resp == 1) {
348-
/* the connection is already released */
349-
break;
350347
} else if (con->state==S_CONN_EOF) {
351348
reactor_del_all( con->fd, idx, IO_FD_CLOSING );
352349
tcpconn_check_del(con);

net/proto_tcp/proto_tcp.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ int tcp_read(struct tcp_connection *c,struct tcp_req *r)
648648
*/
649649
static int tcp_read_req(struct tcp_connection* con, int* bytes_read)
650650
{
651-
int bytes, rc;
651+
int bytes;
652652
int total_bytes;
653653
struct tcp_req* req;
654654

@@ -736,7 +736,7 @@ static int tcp_read_req(struct tcp_connection* con, int* bytes_read)
736736
int max_chunks = tcp_attr_isset(con, TCP_ATTR_MAX_MSG_CHUNKS) ?
737737
con->profile.attrs[TCP_ATTR_MAX_MSG_CHUNKS] : tcp_max_msg_chunks;
738738

739-
switch ((rc = tcp_handle_req(req,con,max_chunks,parallel_handling))){
739+
switch (tcp_handle_req(req,con,max_chunks,parallel_handling)){
740740
case 1:
741741
goto again;
742742
case -1:
@@ -746,9 +746,8 @@ static int tcp_read_req(struct tcp_connection* con, int* bytes_read)
746746
LM_DBG("tcp_read_req end for conn %p, req is %p\n",con,con->con_req);
747747
done:
748748
if (bytes_read) *bytes_read=total_bytes;
749-
750-
return rc == 2 ? 1 /* connection is already released! */
751-
/* 0,1? */: 0; /* connection will be released */
749+
/* connection will be released */
750+
return 0;
752751
error:
753752
/* connection will be released as ERROR */
754753
return -1;

net/proto_tcp/tcp_common.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,6 @@ static inline int tcp_handle_req(struct tcp_req *req,
382382

383383
/* prepare for next request */
384384
size=req->pos-req->parsed;
385-
con->msg_attempts = 0;
386385

387386
if (req->state==H_PING_CRLFCRLF) {
388387
/* we send the reply */
@@ -414,7 +413,6 @@ static inline int tcp_handle_req(struct tcp_req *req,
414413
msg_buf_cpy[msg_len] = 0;
415414
msg_buf = msg_buf_cpy;
416415
tcp_done_reading( con );
417-
con = NULL; /* having reached this, we MUST return 2 */
418416
}
419417

420418
} else {
@@ -430,6 +428,8 @@ static inline int tcp_handle_req(struct tcp_req *req,
430428
pkg_free(msg_buf_cpy);
431429
}
432430

431+
con->msg_attempts = 0;
432+
433433
if (size) {
434434
/* restoring the char only makes sense if there is something else to
435435
* process, otherwise we can leave it. This prevents us from accessing
@@ -450,8 +450,7 @@ static inline int tcp_handle_req(struct tcp_req *req,
450450
/* if we no longer need this tcp_req
451451
* we can free it now */
452452
shm_free(req);
453-
if (con)
454-
con->con_req = NULL;
453+
con->con_req = NULL;
455454
}
456455
} else {
457456
/* request not complete - check the if the thresholds are exceeded */
@@ -510,8 +509,8 @@ static inline int tcp_handle_req(struct tcp_req *req,
510509
}
511510
}
512511

513-
/* everything ok; if connection was returned already, use special rc 2 */
514-
return con ? 0 : 2;
512+
/* everything ok */
513+
return 0;
515514
error:
516515
/* report error */
517516
return -1;

0 commit comments

Comments
 (0)