weimingtom / angel-engine

Automatically exported from code.google.com/p/angel-engine
0 stars 0 forks source link

Message system redesign #83

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Run Angel
2. Send a bunch of messages
3. Lose memory

Message is a base class, found in Message.h, which is inherited by 
TypedMessage, and used polymorphically by theSwitchBoard. However, Message does 
not have a virtual destructor, and TypedMessage stores additional data which 
must be deconstructed. 

Because Message does not have a virtual destructor, any TypedMessage put into 
theSwitchBoard will not call TypedMessage's deconstructor, leading to any 
additional data TypedMessage is storing not being freed. 

The fix is a simple one line addition on line 60 of Message.h:
virtual ~Message(){};

However, there is another issue which is related to this one: TypedMessage must 
be accessed through an unsafe downcast, which is error prone. If a dynamic_cast 
is not used and instead a static_cast or C Style cast is used by mistake (or 
perhaps bravado), it's quite easy for random data to be accessed and returned 
without any sort of error, causing a time bomb.

While the architecture is to blame, there is a simple patch which can fix this 
issue, which I have included. The patch uses RTTI to establish that the type 
stored is the same as the type taken out, aborting or returning a default value 
if this is not the case. 

While this forces the messaging system to use RTTI rather than type erasure or 
some more elegant technique, Angel already relies on RTTI; dynamic_cast is used 
elsewhere with reckless disregard for the fact RTTI may not be available. 
Therefore, this change should not add any new requirements to Angel, does not 
change the interface, and adds a high degree of safety and reliability to an 
otherwise unpredictable interface.

Plus it's only like, three to five additional lines. Surely that's not so large 
a change?

Original issue reported on code.google.com by LoveOver...@gmail.com on 29 Sep 2013 at 6:53

Attachments:

GoogleCodeExporter commented 8 years ago
The virtual destructor has been added in the repo. 

The rest of the issue is focused on design changes which I'd prefer to roll in 
to a larger change to the Message system. 

Original comment by lieseg...@gmail.com on 8 Oct 2013 at 4:40