network: fix segfaults when TCP & UDP blocks are restarted

The TCP and UDP blocks segfault if start() is called after stop(), 
because stop() frees resources that are not re-allocated by start(). To 
fix this, I've moved resource allocation for these blocks from the 
constructor to start().

Signed-off-by: Clayton Smith <argilo@gmail.com>

9 files changed, 172 insertions, 26 deletions

diff --git a/gr-network/lib/tcp_sink_impl.cc b/gr-network/lib/tcp_sink_impl.cc
index cab0d8ab5..7db85d5b1 100644
--- a/gr-network/lib/tcp_sink_impl.cc
+++ b/gr-network/lib/tcp_sink_impl.cc
@@ -50,19 +50,22 @@ tcp_sink_impl::tcp_sink_impl(
       d_initial_connection(true)
 {
     d_block_size = d_itemsize * d_veclen;
+}
 
+bool tcp_sink_impl::start()
+{
     if (d_sinkmode == TCPSINKMODE_CLIENT) {
         // In this mode, we're connecting to a remote TCP service listener
         // as a client.
         std::stringstream msg;
 
-        msg << "[TCP Sink] connecting to " << host << " on port " << port;
+        msg << "[TCP Sink] connecting to " << d_host << " on port " << d_port;
         GR_LOG_INFO(d_logger, msg.str());
 
         boost::system::error_code err;
         d_tcpsocket = new boost::asio::ip::tcp::socket(d_io_service);
 
-        std::string s_port = (boost::format("%d") % port).str();
+        std::string s_port = (boost::format("%d") % d_port).str();
         boost::asio::ip::tcp::resolver resolver(d_io_service);
         boost::asio::ip::tcp::resolver::query query(
             d_host, s_port, boost::asio::ip::resolver_query_base::passive);
@@ -101,6 +104,8 @@ tcp_sink_impl::tcp_sink_impl(
         d_start_new_listener = true;
         d_listener_thread = new boost::thread([this] { run_listener(); });
     }
+
+    return true;
 }
 
 void tcp_sink_impl::run_listener()
diff --git a/gr-network/lib/tcp_sink_impl.h b/gr-network/lib/tcp_sink_impl.h
index caa2e78e6..3a3c0dfab 100644
--- a/gr-network/lib/tcp_sink_impl.h
+++ b/gr-network/lib/tcp_sink_impl.h
@@ -57,6 +57,7 @@ public:
                   int sinkmode = TCPSINKMODE_CLIENT);
     ~tcp_sink_impl() override;
 
+    bool start() override;
     bool stop() override;
 
     void accept_handler(boost::asio::ip::tcp::socket* new_connection,
diff --git a/gr-network/lib/udp_sink_impl.cc b/gr-network/lib/udp_sink_impl.cc
index 3e5ccad70..95fb39f7b 100644
--- a/gr-network/lib/udp_sink_impl.cc
+++ b/gr-network/lib/udp_sink_impl.cc
@@ -45,6 +45,8 @@ udp_sink_impl::udp_sink_impl(size_t itemsize,
     : gr::sync_block("udp_sink",
                      gr::io_signature::make(1, 1, itemsize * veclen),
                      gr::io_signature::make(0, 0, 0)),
+      d_host(host),
+      d_port(port),
       d_itemsize(itemsize),
       d_veclen(veclen),
       d_header_type(header_type),
@@ -62,10 +64,6 @@ udp_sink_impl::udp_sink_impl(size_t itemsize,
     // size is 8972 (9000-the UDP 28-byte header) Same rules apply with
     // fragmentation.
 
-    d_port = port;
-
-    d_header_size = 0;
-
     switch (d_header_type) {
     case HEADERTYPE_SEQNUM:
         d_header_size = sizeof(header_seq_num);
@@ -94,13 +92,21 @@ udp_sink_impl::udp_sink_impl(size_t itemsize,
             "least 8 bytes once header/trailer adjustments are made.");
     }
 
-    d_seq_num = 0;
-
     d_block_size = d_itemsize * d_veclen;
 
     d_precomp_datasize = d_payloadsize - d_header_size;
     d_precomp_data_overitemsize = d_precomp_datasize / d_itemsize;
 
+    int out_multiple = (d_payloadsize - d_header_size) / d_block_size;
+
+    if (out_multiple == 1)
+        out_multiple = 2; // Ensure we get pairs, for instance complex -> ichar pairs
+
+    gr::block::set_output_multiple(out_multiple);
+}
+
+bool udp_sink_impl::start()
+{
     d_localbuffer = new char[d_payloadsize];
 
     long max_circ_buffer;
@@ -119,8 +125,8 @@ udp_sink_impl::udp_sink_impl(size_t itemsize,
 
     d_udpsocket = new boost::asio::ip::udp::socket(d_io_service);
 
-    std::string str_port = (boost::format("%d") % port).str();
-    std::string str_host = host.empty() ? std::string("localhost") : host;
+    std::string str_port = (boost::format("%d") % d_port).str();
+    std::string str_host = d_host.empty() ? std::string("localhost") : d_host;
     boost::asio::ip::udp::resolver resolver(d_io_service);
     boost::asio::ip::udp::resolver::query query(
         str_host, str_port, boost::asio::ip::resolver_query_base::passive);
@@ -133,7 +139,7 @@ udp_sink_impl::udp_sink_impl(size_t itemsize,
                                  err.message());
     }
 
-    if (host.find(":") != std::string::npos)
+    if (d_host.find(":") != std::string::npos)
         is_ipv6 = true;
     else {
         // This block supports a check that a name rather than an IP is provided.
@@ -150,12 +156,7 @@ udp_sink_impl::udp_sink_impl(size_t itemsize,
         d_udpsocket->open(boost::asio::ip::udp::v4());
     }
 
-    int out_multiple = (d_payloadsize - d_header_size) / d_block_size;
-
-    if (out_multiple == 1)
-        out_multiple = 2; // Ensure we get pairs, for instance complex -> ichar pairs
-
-    gr::block::set_output_multiple(out_multiple);
+    return true;
 }
 
 /*
diff --git a/gr-network/lib/udp_sink_impl.h b/gr-network/lib/udp_sink_impl.h
index 3e5f3867d..089cb6d33 100644
--- a/gr-network/lib/udp_sink_impl.h
+++ b/gr-network/lib/udp_sink_impl.h
@@ -24,6 +24,7 @@ namespace network {
 class NETWORK_API udp_sink_impl : public udp_sink
 {
 protected:
+    std::string d_host;
     int d_port;
     size_t d_itemsize;
     size_t d_veclen;
@@ -67,6 +68,7 @@ public:
                   bool send_eof = true);
     ~udp_sink_impl() override;
 
+    bool start() override;
     bool stop() override;
 
     int work(int noutput_items,
diff --git a/gr-network/lib/udp_source_impl.cc b/gr-network/lib/udp_source_impl.cc
index a05f6c103..259506703 100644
--- a/gr-network/lib/udp_source_impl.cc
+++ b/gr-network/lib/udp_source_impl.cc
@@ -103,6 +103,16 @@ udp_source_impl::udp_source_impl(size_t itemsize,
     d_precomp_data_size = d_payloadsize - d_header_size;
     d_precomp_data_over_item_size = d_precomp_data_size / d_itemsize;
 
+    int out_multiple = (d_payloadsize - d_header_size) / d_block_size;
+
+    if (out_multiple == 1)
+        out_multiple = 2; // Ensure we get pairs, for instance complex -> ichar pairs
+
+    gr::block::set_output_multiple(out_multiple);
+}
+
+bool udp_source_impl::start()
+{
     d_local_buffer = new char[d_payloadsize];
     long max_circ_buffer;
 
@@ -119,9 +129,9 @@ udp_source_impl::udp_source_impl(size_t itemsize,
     d_localqueue = new boost::circular_buffer<char>(max_circ_buffer);
 
     if (is_ipv6)
-        d_endpoint = boost::asio::ip::udp::endpoint(boost::asio::ip::udp::v6(), port);
+        d_endpoint = boost::asio::ip::udp::endpoint(boost::asio::ip::udp::v6(), d_port);
     else
-        d_endpoint = boost::asio::ip::udp::endpoint(boost::asio::ip::udp::v4(), port);
+        d_endpoint = boost::asio::ip::udp::endpoint(boost::asio::ip::udp::v4(), d_port);
 
     try {
         d_udpsocket = new boost::asio::ip::udp::socket(d_io_service, d_endpoint);
@@ -130,16 +140,12 @@ udp_source_impl::udp_source_impl(size_t itemsize,
                                  ex.what());
     }
 
-    int out_multiple = (d_payloadsize - d_header_size) / d_block_size;
-
-    if (out_multiple == 1)
-        out_multiple = 2; // Ensure we get pairs, for instance complex -> ichar pairs
-
-    gr::block::set_output_multiple(out_multiple);
 
     std::stringstream msg_stream;
-    msg_stream << "Listening for data on UDP port " << port << ".";
+    msg_stream << "Listening for data on UDP port " << d_port << ".";
     GR_LOG_INFO(d_logger, msg_stream.str());
+
+    return true;
 }
 
 /*
diff --git a/gr-network/lib/udp_source_impl.h b/gr-network/lib/udp_source_impl.h
index b11835f17..8b10819c9 100644
--- a/gr-network/lib/udp_source_impl.h
+++ b/gr-network/lib/udp_source_impl.h
@@ -69,6 +69,7 @@ public:
                     bool ipv6);
     ~udp_source_impl() override;
 
+    bool start() override;
     bool stop() override;
 
     size_t data_available();
diff --git a/gr-network/python/network/qa_tcp_sink.py b/gr-network/python/network/qa_tcp_sink.py
new file mode 100644
index 000000000..091827867
--- /dev/null
+++ b/gr-network/python/network/qa_tcp_sink.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+#
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This file is part of GNU Radio
+#
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+
+from gnuradio import gr, gr_unittest, blocks, network
+import socket
+import threading
+import time
+
+
+class qa_tcp_sink (gr_unittest.TestCase):
+    def tcp_receive(self, serversocket):
+        for _ in range(2):
+            clientsocket, address = serversocket.accept()
+            while True:
+                data = clientsocket.recv(4096)
+                if not data:
+                    break
+            clientsocket.close()
+
+    def setUp(self):
+        self.tb = gr.top_block()
+
+    def tearDown(self):
+        self.tb = None
+
+    def test_restart(self):
+        serversocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        serversocket.bind(('localhost', 2000))
+        serversocket.listen()
+
+        thread = threading.Thread(target=self.tcp_receive, args=(serversocket,))
+        thread.start()
+
+        null_source = blocks.null_source(gr.sizeof_gr_complex)
+        throttle = blocks.throttle(gr.sizeof_gr_complex, 320000, True)
+        tcp_sink = network.tcp_sink(gr.sizeof_gr_complex, 1, '127.0.0.1', 2000, 1)
+        self.tb.connect(null_source, throttle, tcp_sink)
+        self.tb.start()
+        time.sleep(0.1)
+        self.tb.stop()
+        time.sleep(0.1)
+        self.tb.start()
+        time.sleep(0.1)
+        self.tb.stop()
+
+        thread.join()
+        serversocket.close()
+
+
+if __name__ == '__main__':
+    gr_unittest.run(qa_tcp_sink)
diff --git a/gr-network/python/network/qa_udp_sink.py b/gr-network/python/network/qa_udp_sink.py
new file mode 100644
index 000000000..8c88920d2
--- /dev/null
+++ b/gr-network/python/network/qa_udp_sink.py
@@ -0,0 +1,37 @@
+#!/usr/bin/env python
+#
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This file is part of GNU Radio
+#
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+
+from gnuradio import gr, gr_unittest, blocks, network
+import time
+
+
+class qa_udp_sink (gr_unittest.TestCase):
+    def setUp(self):
+        self.tb = gr.top_block()
+
+    def tearDown(self):
+        self.tb = None
+
+    def test_restart(self):
+        null_source = blocks.null_source(gr.sizeof_gr_complex)
+        throttle = blocks.throttle(gr.sizeof_gr_complex, 320000, True)
+        udp_sink = network.udp_sink(gr.sizeof_gr_complex, 1, '127.0.0.1', 2000,
+                                    0, 1472, False)
+        self.tb.connect(null_source, throttle, udp_sink)
+        self.tb.start()
+        time.sleep(0.1)
+        self.tb.stop()
+        time.sleep(0.1)
+        self.tb.start()
+        time.sleep(0.1)
+        self.tb.stop()
+
+
+if __name__ == '__main__':
+    gr_unittest.run(qa_udp_sink)
diff --git a/gr-network/python/network/qa_udp_source.py b/gr-network/python/network/qa_udp_source.py
new file mode 100644
index 000000000..67c76da99
--- /dev/null
+++ b/gr-network/python/network/qa_udp_source.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
+#
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This file is part of GNU Radio
+#
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+
+from gnuradio import gr, gr_unittest, blocks, network
+import time
+
+
+class qa_udp_source (gr_unittest.TestCase):
+    def setUp(self):
+        self.tb = gr.top_block()
+
+    def tearDown(self):
+        self.tb = None
+
+    def test_restart(self):
+        udp_source = network.udp_source(gr.sizeof_gr_complex, 1, 1234, 0, 1472,
+                                        False, False, False)
+        null_sink = blocks.null_sink(gr.sizeof_gr_complex)
+        self.tb.connect(udp_source, null_sink)
+        self.tb.start()
+        time.sleep(0.1)
+        self.tb.stop()
+        time.sleep(0.1)
+        self.tb.start()
+        time.sleep(0.1)
+        self.tb.stop()
+
+
+if __name__ == '__main__':
+    gr_unittest.run(qa_udp_source)