GetNode is bad in multithreaded devices
authorCraig Dowell <craigdo@ee.washington.edu>
Fri, 19 Feb 2010 14:05:30 -0800
changeset 6004 2ab92cdbaafa
parent 6003 d6c026abfb3f
child 6005 ea5ac3f29800
GetNode is bad in multithreaded devices
src/devices/emu/emu-net-device.cc
src/devices/emu/emu-net-device.h
src/devices/tap-bridge/tap-bridge.cc
src/devices/tap-bridge/tap-bridge.h
--- a/src/devices/emu/emu-net-device.cc	Fri Feb 19 12:25:50 2010 +0100
+++ b/src/devices/emu/emu-net-device.cc	Fri Feb 19 14:05:30 2010 -0800
@@ -272,6 +272,14 @@
   Ptr<RealtimeSimulatorImpl> impl = DynamicCast<RealtimeSimulatorImpl> (Simulator::GetImplementation ());
   m_rtImpl = GetPointer (impl);
 
+  //
+  // A similar story exists for the node ID.  We can't just naively do a
+  // GetNode ()->GetId () since GetNode is going to give us a Ptr<Node> which
+  // is reference counted.  We need to stash away the node ID for use in the
+  // read thread.
+  //
+  m_nodeId = GetNode ()->GetId ();
+
   NS_LOG_LOGIC ("Creating socket");
 
   //
@@ -757,10 +765,10 @@
           return;
         }
 
-      NS_LOG_INFO ("EmuNetDevice::ReadThread(): Received packet");
+      NS_LOG_INFO ("EmuNetDevice::EmuNetDevice(): Received packet on node " << m_nodeId);
       NS_LOG_INFO ("EmuNetDevice::ReadThread(): Scheduling handler");
       NS_ASSERT_MSG (m_rtImpl, "EmuNetDevice::ReadThread(): Realtime simulator implementation pointer not set");
-      m_rtImpl->ScheduleRealtimeNowWithContext (GetNode ()->GetId (), MakeEvent (&EmuNetDevice::ForwardUp, this, buf, len));
+      m_rtImpl->ScheduleRealtimeNowWithContext (m_nodeId, MakeEvent (&EmuNetDevice::ForwardUp, this, buf, len));
       buf = 0;
     }
 }
--- a/src/devices/emu/emu-net-device.h	Fri Feb 19 12:25:50 2010 +0100
+++ b/src/devices/emu/emu-net-device.h	Fri Feb 19 14:05:30 2010 -0800
@@ -510,6 +510,13 @@
    * Never free this pointer!
    */
   RealtimeSimulatorImpl *m_rtImpl;
+
+  /*
+   * a copy of the node id so the read thread doesn't have to GetNode() in
+   * in order to find the node ID.  Thread unsafe reference counting in 
+   * multithreaded apps is not a good thing.
+   */
+  uint32_t m_nodeId;
 };
 
 } // namespace ns3
--- a/src/devices/tap-bridge/tap-bridge.cc	Fri Feb 19 12:25:50 2010 +0100
+++ b/src/devices/tap-bridge/tap-bridge.cc	Fri Feb 19 14:05:30 2010 -0800
@@ -206,6 +206,14 @@
   m_rtImpl = GetPointer (impl);
 
   //
+  // A similar story exists for the node ID.  We can't just naively do a
+  // GetNode ()->GetId () since GetNode is going to give us a Ptr<Node> which
+  // is reference counted.  We need to stash away the node ID for use in the
+  // read thread.
+  //
+  m_nodeId = GetNode ()->GetId ();
+
+  //
   // Spin up the tap bridge and start receiving packets.
   //
   NS_LOG_LOGIC ("Creating tap device");
@@ -648,11 +656,10 @@
           return;
         }
 
-      NS_LOG_INFO ("TapBridge::ReadThread(): Received packet on node " << m_node->GetId ());
+      NS_LOG_INFO ("TapBridge::ReadThread(): Received packet on node " << m_nodeId);
       NS_LOG_INFO ("TapBridge::ReadThread(): Scheduling handler");
       NS_ASSERT_MSG (m_rtImpl, "EmuNetDevice::ReadThread(): Realtime simulator implementation pointer not set");
-      m_rtImpl->ScheduleRealtimeNowWithContext (GetNode ()->GetId (),
-        MakeEvent (&TapBridge::ForwardToBridgedDevice, this, buf, len));
+      m_rtImpl->ScheduleRealtimeNowWithContext (m_nodeId, MakeEvent (&TapBridge::ForwardToBridgedDevice, this, buf, len));
       buf = 0;
     }
 }
--- a/src/devices/tap-bridge/tap-bridge.h	Fri Feb 19 12:25:50 2010 +0100
+++ b/src/devices/tap-bridge/tap-bridge.h	Fri Feb 19 14:05:30 2010 -0800
@@ -457,6 +457,13 @@
    * Never free this pointer!
    */
   RealtimeSimulatorImpl *m_rtImpl;
+
+  /*
+   * a copy of the node id so the read thread doesn't have to GetNode() in
+   * in order to find the node ID.  Thread unsafe reference counting in 
+   * multithreaded apps is not a good thing.
+   */
+  uint32_t m_nodeId;
 };
 
 } // namespace ns3