Changeset 2070 for code/branches/objecthierarchy/src/network/packet/Packet.cc

Timestamp:

Oct 31, 2008, 12:05:12 AM (16 years ago)

Author:

rgrieder

Message:

Properly took care of network::packet::Packet destruction. It depends very much whether the the data was allocated by ENet or by ourselves.

File:

• code/branches/objecthierarchy/src/network/packet/Packet.cc (modified) (6 diffs)

code/branches/objecthierarchy/src/network/packet/Packet.cc

@@ -61 +61 @@
   data_=0;
   enetPacket_=0;
+  bDataENetAllocated_ = false;
 }
 
@@ -73 +74 @@
   data_=data;
   enetPacket_=0;
+  bDataENetAllocated_ = false;
 }
 
@@ -86 +88 @@
   }else
     data_=0;
-}
-
+  bDataENetAllocated_ = p.bDataENetAllocated_;
+}
+
+/**
+@brief
+    Destroys a packet completely.
+    
+    That also means destroying the ENetPacket if one exists. There 
+*/
 Packet::~Packet(){
-  if(enetPacket_){
-    assert(enetPacket_->freeCallback==0);
+  // Deallocate data_ memory if necessary.
+  if (this->bDataENetAllocated_){
+    // In this case ENet allocated data_ and will destroy it.
+  }
+  else if (this->data_) {
+    // This destructor was probably called as a consequence to ENet executing our callback.
+    // It simply serves us to be able to deallocate the packet content (data_) ourselves since
+    // we have created it in the first place.
+    delete[] this->data_;
+  }
+
+  // Destroy the ENetPacket if necessary.
+  // Note: For the case ENet used the callback to destroy the packet, we have already set
+  // enetPacket_ to NULL to avoid destroying it again.
+  if (this->enetPacket_){
+    // enetPacket_->data gets destroyed too by ENet if it was allocated by it.
     enet_packet_destroy(enetPacket_);
   }
-  if(data_)
-    delete[] data_;
 }
 
@@ -107 +128 @@
       return false;
     }
+    // We deliver ENet the data address so that it doesn't memcpy everything again.
+    // --> We have to delete data_ ourselves!
     enetPacket_ = enet_packet_create(getData(), getSize(), getFlags());
     enetPacket_->freeCallback = &Packet::deletePacket;
-//     enetPacket_->freeCallback = &blub;
+    // Add the packet to a global list so we can access it again once enet calls our
+    // deletePacket method. We can of course only give a one argument function to the ENet C library.
     packetMap_[enetPacket_] = this;
   }
@@ -127 +151 @@
   }
 #endif
-  ENetPacket *temp = enetPacket_;
-  enetPacket_ = 0; // otherwise we have a double free because enet already handles the deallocation of the packet
-  network::Host::addPacket( temp, clientID_);
+//  ENetPacket *temp = enetPacket_;
+//  enetPacket_ = 0; // otherwise we have a double free because enet already handles the deallocation of the packet
+  network::Host::addPacket( enetPacket_, clientID_);
   return true;
 }
@@ -170 +194 @@
       break;
   }
+
+  // Data was created by ENet
+  p->bDataENetAllocated_ = true;
+
   return p;
 }
 
-void Packet::deletePacket(ENetPacket *packet){
-  assert(packetMap_[packet]);
-  assert(packetMap_[packet]->enetPacket_==0);
-  delete packetMap_[packet];
+/**
+@brief
+    ENet calls this method whenever it wants to destroy a packet that contains
+    data we allocated ourselves.
+*/
+void Packet::deletePacket(ENetPacket *enetPacket){
+  // Get our Packet from a gloabal map with all Packets created in the send() method of Packet.
+  std::map<ENetPacket*, Packet*>::iterator it = packetMap_.find(enetPacket);
+  assert(it != packetMap_.end());
+  // Make sure we don't delete it again in the destructor
+  it->second->enetPacket_ = 0;
+  delete it->second;
+  packetMap_.erase(it);
 }