device3: Fixed potential concurreny issues

- Protected block_ctrl vector with a mutex
  - Note: const block accessors are not thread safe
- Removed sid_framer from base device3 class
- Made x300,e300 sid_framers atomic

7 files changed, 13 insertions, 16 deletions

diff --git a/host/lib/device3.cpp b/host/lib/device3.cpp
index c3bfb83d2..b2f27e744 100644
--- a/host/lib/device3.cpp
+++ b/host/lib/device3.cpp
@@ -68,6 +68,7 @@ std::vector<rfnoc::block_id_t> device3::find_blocks(const std::string &block_id_
 
 void device3::clear()
 {
+    boost::lock_guard<boost::mutex> lock(_block_ctrl_mutex);
     for(const block_ctrl_base::sptr &block:  _rfnoc_block_ctrl) {
         block->clear();
     }
diff --git a/host/lib/usrp/device3/device3_impl.cpp b/host/lib/usrp/device3/device3_impl.cpp
index f680e5a74..35faf601f 100644
--- a/host/lib/usrp/device3/device3_impl.cpp
+++ b/host/lib/usrp/device3/device3_impl.cpp
@@ -28,7 +28,6 @@
 using namespace uhd::usrp;
 
 device3_impl::device3_impl()
-    : _sid_framer(0)
 {
     _type = uhd::device::USRP;
     _async_md.reset(new async_md_type(1000/*messages deep*/));
@@ -172,7 +171,10 @@ void device3_impl::enumerate_rfnoc_blocks(
         make_args.base_address = xport.send_sid.get_dst();
         make_args.device_index = device_index;
         make_args.tree = subtree;
-        _rfnoc_block_ctrl.push_back(uhd::rfnoc::block_ctrl_base::make(make_args, noc_id));
+        {   //Critical section for block_ctrl vector access
+            boost::lock_guard<boost::mutex> lock(_block_ctrl_mutex);
+            _rfnoc_block_ctrl.push_back(uhd::rfnoc::block_ctrl_base::make(make_args, noc_id));
+        }
     }
 }
 
diff --git a/host/lib/usrp/device3/device3_impl.hpp b/host/lib/usrp/device3/device3_impl.hpp
index c496b5105..c2ec26f80 100644
--- a/host/lib/usrp/device3/device3_impl.hpp
+++ b/host/lib/usrp/device3/device3_impl.hpp
@@ -172,9 +172,6 @@ protected:
     /***********************************************************************
      * Members
      **********************************************************************/
-    //! A counter, designed to create unique SIDs
-    size_t _sid_framer;
-
     // TODO: Maybe move these to private
     uhd::dict<std::string, boost::weak_ptr<uhd::rx_streamer> > _rx_streamers;
     uhd::dict<std::string, boost::weak_ptr<uhd::tx_streamer> > _tx_streamers;
diff --git a/host/lib/usrp/e300/e300_impl.cpp b/host/lib/usrp/e300/e300_impl.cpp
index ea326878e..20dd89dd1 100644
--- a/host/lib/usrp/e300/e300_impl.cpp
+++ b/host/lib/usrp/e300/e300_impl.cpp
@@ -747,15 +747,16 @@ uint32_t e300_impl::_allocate_sid(const sid_config_t &config)
 {
     const uint32_t stream = (config.dst_prefix | (config.router_dst_there << 2)) & 0xff;
 
+    const size_t sid_framer = _sid_framer++; //increment for next setup
     const uint32_t sid = 0
         | (E300_DEVICE_HERE << 24)
-        | (_sid_framer << 16)
+        | (sid_framer << 16)
         | (config.router_addr_there << 8)
         | (stream << 0)
     ;
     UHD_LOGGER_DEBUG("E300")<< std::hex
         << " sid 0x" << sid
-        << " framer 0x" << _sid_framer
+        << " framer 0x" << sid_framer
         << " stream 0x" << stream
         << " router_dst_there 0x" << int(config.router_dst_there)
         << " router_addr_there 0x" << int(config.router_addr_there)
@@ -779,9 +780,6 @@ uint32_t e300_impl::_allocate_sid(const sid_config_t &config)
         << "done router config for sid 0x" << sid
         << std::dec ;
 
-    //increment for next setup
-    _sid_framer++;
-
     return sid;
 }
 
diff --git a/host/lib/usrp/e300/e300_impl.hpp b/host/lib/usrp/e300/e300_impl.hpp
index 50d78fdd4..cc2e39e23 100644
--- a/host/lib/usrp/e300/e300_impl.hpp
+++ b/host/lib/usrp/e300/e300_impl.hpp
@@ -47,6 +47,7 @@
 #include "e300_i2c.hpp"
 #include "e300_eeprom_manager.hpp"
 #include "e300_sensor_manager.hpp"
+#include <atomic>
 
 /* if we don't compile with gpsd support, don't bother */
 #ifdef E300_GPSD
@@ -288,7 +289,7 @@ private: // members
     uhd::device_addr_t                     _device_addr;
     xport_t                                _xport_path;
     e300_fifo_interface::sptr              _fifo_iface;
-    size_t                                 _sid_framer;
+    std::atomic<size_t>                    _sid_framer;
     radio_perifs_t                         _radio_perifs[2];
     double                                 _tick_rate;
     ad9361_ctrl::sptr                      _codec_ctrl;
diff --git a/host/lib/usrp/x300/x300_impl.cpp b/host/lib/usrp/x300/x300_impl.cpp
index c9a73db16..8e48c606a 100644
--- a/host/lib/usrp/x300/x300_impl.cpp
+++ b/host/lib/usrp/x300/x300_impl.cpp
@@ -1307,7 +1307,7 @@ uhd::sid_t x300_impl::allocate_sid(
 ) {
     uhd::sid_t sid = address;
     sid.set_src_addr(src_addr);
-    sid.set_src_endpoint(_sid_framer);
+    sid.set_src_endpoint(_sid_framer++);    //increment for next setup
 
     // TODO Move all of this setup_mb()
     // Program the X300 to recognise it's own local address.
@@ -1321,9 +1321,6 @@ uhd::sid_t x300_impl::allocate_sid(
 
     UHD_LOGGER_DEBUG("X300") << "done router config for sid " << sid ;
 
-    //increment for next setup
-    _sid_framer++;
-
     return sid;
 }
 
diff --git a/host/lib/usrp/x300/x300_impl.hpp b/host/lib/usrp/x300/x300_impl.hpp
index 7bb624577..2de295bd9 100644
--- a/host/lib/usrp/x300/x300_impl.hpp
+++ b/host/lib/usrp/x300/x300_impl.hpp
@@ -39,6 +39,7 @@
 #include <uhd/rfnoc/block_ctrl.hpp>
 ///////////// RFNOC /////////////////////
 #include <boost/dynamic_bitset.hpp>
+#include <atomic>
 
 static const std::string X300_FW_FILE_NAME  = "usrp_x300_fw.bin";
 static const std::string X300_DEFAULT_CLOCK_SOURCE  = "internal";
@@ -215,7 +216,7 @@ private:
     //task for periodically reclaiming the device from others
     void claimer_loop(uhd::wb_iface::sptr);
 
-    size_t _sid_framer;
+    std::atomic<size_t> _sid_framer;
 
     uhd::sid_t allocate_sid(
         mboard_members_t &mb,