From: W.C.A. Wijngaards Date: Wed, 25 Nov 2020 12:46:28 +0000 (+0100) Subject: - Fix readagain and writeagain callback functions for comm point X-Git-Tag: release-1.13.0rc2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e50152aa1f23f83a981479f7f31039f8bf2f28fc;p=thirdparty%2Funbound.git - Fix readagain and writeagain callback functions for comm point cleanup. --- diff --git a/doc/Changelog b/doc/Changelog index c7b2a1b9a..0e2c254dd 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -7,6 +7,8 @@ - Fix memory leak for edns client tag opcode config element. - Attempt fix for libevent state in tcp reuse cases after a packet is written. + - Fix readagain and writeagain callback functions for comm point + cleanup. 24 November 2020: Wouter - Merge PR #283 : Stream reuse. This implements upstream stream diff --git a/services/outside_network.c b/services/outside_network.c index 7b5dc8459..84876ff3f 100644 --- a/services/outside_network.c +++ b/services/outside_network.c @@ -655,6 +655,10 @@ outnet_tcp_take_into_use(struct waiting_tcp* w) pend->query = w; pend->reuse.outnet = w->outnet; pend->c->repinfo.addrlen = w->addrlen; + pend->c->tcp_more_read_again = &pend->reuse.cp_more_read_again; + pend->c->tcp_more_write_again = &pend->reuse.cp_more_write_again; + pend->reuse.cp_more_read_again = 0; + pend->reuse.cp_more_write_again = 0; memcpy(&pend->c->repinfo.addr, &w->addr, w->addrlen); pend->reuse.pending = pend; if(pend->c->ssl) @@ -804,8 +808,8 @@ reuse_move_writewait_away(struct outside_network* outnet, pend->c->tcp_write_pkt = NULL; pend->c->tcp_write_pkt_len = 0; pend->c->tcp_write_and_read = 0; - pend->c->tcp_more_read_again = 0; - pend->c->tcp_more_write_again = 0; + pend->reuse.cp_more_read_again = 0; + pend->reuse.cp_more_write_again = 0; pend->c->tcp_is_reading = 1; w = pend->query; pend->query = NULL; @@ -1008,7 +1012,7 @@ outnet_tcp_cb(struct comm_point* c, void* arg, int error, * because this callback called after a tcp write * succeeded and likely more buffer space is available * and we can write some more. */ - pend->c->tcp_more_write_again = 1; + pend->reuse.cp_more_write_again = 1; pend->query = reuse_write_wait_pop(&pend->reuse); comm_point_stop_listening(pend->c); outnet_tcp_take_query_setup(pend->c->fd, pend, @@ -1016,8 +1020,8 @@ outnet_tcp_cb(struct comm_point* c, void* arg, int error, } else { verbose(VERB_ALGO, "outnet tcp writes done, wait"); pend->c->tcp_write_and_read = 0; - pend->c->tcp_more_read_again = 0; - pend->c->tcp_more_write_again = 0; + pend->reuse.cp_more_read_again = 0; + pend->reuse.cp_more_write_again = 0; pend->c->tcp_is_reading = 1; comm_point_stop_listening(pend->c); reuse_tcp_setup_timeout(pend); @@ -1072,7 +1076,7 @@ outnet_tcp_cb(struct comm_point* c, void* arg, int error, * because this callback called after a successful read * and there could be more bytes to read on the input */ if(pend->reuse.tree_by_id.count != 0) - pend->c->tcp_more_read_again = 1; + pend->reuse.cp_more_read_again = 1; reuse_tcp_setup_read_and_timeout(pend); return 0; } diff --git a/services/outside_network.h b/services/outside_network.h index 48f9d3f03..2fe97fa6c 100644 --- a/services/outside_network.h +++ b/services/outside_network.h @@ -266,6 +266,18 @@ struct reuse_tcp { * or not is also part of the key to the rbtree. * There is a timeout and read event on the fd, to close it. */ struct pending_tcp* pending; + /** + * The more read again value pointed to by the commpoint + * tcp_more_read_again pointer, so that it exists after commpoint + * delete + */ + int cp_more_read_again; + /** + * The more write again value pointed to by the commpoint + * tcp_more_write_again pointer, so that it exists after commpoint + * delete + */ + int cp_more_write_again; /** rbtree with other queries waiting on the connection, by ID number, * of type struct waiting_tcp. It is for looking up received * answers to the structure for callback. And also to see if ID diff --git a/util/netevent.c b/util/netevent.c index 814ab6f0a..6bb51cc07 100644 --- a/util/netevent.c +++ b/util/netevent.c @@ -1056,8 +1056,8 @@ reclaim_tcp_handler(struct comm_point* c) comm_point_start_listening(c->tcp_parent, -1, -1); } } - c->tcp_more_read_again = 0; - c->tcp_more_write_again = 0; + c->tcp_more_read_again = NULL; + c->tcp_more_write_again = NULL; } /** do the callback when writing is done */ @@ -1937,8 +1937,9 @@ tcp_more_read_again(int fd, struct comm_point* c) * the connection, the callback signals this, and we try again */ /* this continues until the read routines get EAGAIN or so, * and thus does not call the callback, and the bool is 0 */ - while(c->tcp_more_read_again) { - c->tcp_more_read_again = 0; + int* moreread = c->tcp_more_read_again; + while(moreread && *moreread) { + *moreread = 0; if(!comm_point_tcp_handle_read(fd, c, 0)) { reclaim_tcp_handler(c); if(!c->tcp_do_close) { @@ -1960,8 +1961,9 @@ tcp_more_write_again(int fd, struct comm_point* c) * the callback signals it and we try again. */ /* this continues until the write routines get EAGAIN or so, * and thus does not call the callback, and the bool is 0 */ - while(c->tcp_more_write_again) { - c->tcp_more_write_again = 0; + int* morewrite = c->tcp_more_write_again; + while(morewrite && *morewrite) { + *morewrite = 0; if(!comm_point_tcp_handle_write(fd, c)) { reclaim_tcp_handler(c); if(!c->tcp_do_close) { @@ -2015,6 +2017,7 @@ comm_point_tcp_handle_callback(int fd, short event, void* arg) } if(event&UB_EV_READ) { int has_tcpq = (c->tcp_req_info != NULL); + int* moreread = c->tcp_more_read_again; if(!comm_point_tcp_handle_read(fd, c, 0)) { reclaim_tcp_handler(c); if(!c->tcp_do_close) { @@ -2026,12 +2029,13 @@ comm_point_tcp_handle_callback(int fd, short event, void* arg) } if(has_tcpq && c->tcp_req_info && c->tcp_req_info->read_again) tcp_req_info_read_again(fd, c); - if(c->tcp_more_read_again) + if(moreread && *moreread) tcp_more_read_again(fd, c); return; } if(event&UB_EV_WRITE) { int has_tcpq = (c->tcp_req_info != NULL); + int* morewrite = c->tcp_more_write_again; if(!comm_point_tcp_handle_write(fd, c)) { reclaim_tcp_handler(c); if(!c->tcp_do_close) { @@ -2043,7 +2047,7 @@ comm_point_tcp_handle_callback(int fd, short event, void* arg) } if(has_tcpq && c->tcp_req_info && c->tcp_req_info->read_again) tcp_req_info_read_again(fd, c); - if(c->tcp_more_write_again) + if(morewrite && *morewrite) tcp_more_write_again(fd, c); return; } diff --git a/util/netevent.h b/util/netevent.h index 75baf2177..daa954b64 100644 --- a/util/netevent.h +++ b/util/netevent.h @@ -299,13 +299,23 @@ struct comm_point { /** if set try to read another packet again (over connection with * multiple packets), once set, tries once, then zero again, - * so set it in the packet complete section. */ - int tcp_more_read_again; + * so set it in the packet complete section. + * The pointer itself has to be set before the callback is invoked, + * when you set things up, and continue to exist also after the + * commpoint is closed and deleted in your callback. So that after + * the callback cleans up netevent can see what it has to do. + * Or leave NULL if it is not used at all. */ + int* tcp_more_read_again; /** if set try to write another packet (over connection with * multiple packets), once set, tries once, then zero again, - * so set it in the packet complete section. */ - int tcp_more_write_again; + * so set it in the packet complete section. + * The pointer itself has to be set before the callback is invoked, + * when you set things up, and continue to exist also after the + * commpoint is closed and deleted in your callback. So that after + * the callback cleans up netevent can see what it has to do. + * Or leave NULL if it is not used at all. */ + int* tcp_more_write_again; /** if set, read/write completes: read/write state of tcp is toggled.