Skip to content

Commit

Permalink
Fix Packet memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
@endrift
endrift committed Mar 18, 2011
1 parent 1a061e3 commit 0cc6612
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 8 deletions.
13 changes: 7 additions & 6 deletions client/Client.cpp
View file
Expand Up @@ -335,21 +335,22 @@ void Client::connect(const IPAddress& server_address) {
}

void Client::player_update(const Packet& p) {
Packet::PlayerUpdate update = Packet::PlayerUpdate(p.player_update);
Player* player = get_player(update.player_id);
// XXX don't copy packet
Packet update = Packet(p);
Player* player = get_player(update.player_update.player_id);
if (player == NULL) {
return;
}

// Don't let the server tell us which weapon our own player is using.
if (update.player_id == m_player_id) {
update.current_weapon_id = player->get_current_weapon_id();
if (update.player_update.player_id == m_player_id) {
update.player_update.current_weapon_id = player->get_current_weapon_id();
}

player->read_player_update(update);
player->read_player_update(update.player_update);

// XXX if Weapon::select ever has side effects, we need to have a different way of updating the other players' weapons
Weapon* weapon = get_game()->get_weapon(update.current_weapon_id);
Weapon* weapon = get_game()->get_weapon(update.player_update.current_weapon_id);
if (weapon != NULL) {
weapon->select(player);
}
Expand Down
1 change: 1 addition & 0 deletions client/ClientNetwork.cpp
View file
Expand Up @@ -96,6 +96,7 @@ void ClientNetwork::receive_packets() {
// Keep receiving packets for as long as we can.
while (receive_raw_packet(packet.raw)) {
if (is_connected() && packet.raw.get_address() == m_server_address) {
packet.free();
packet.unmarshal();
if (packet.type != PLAYER_UPDATE_PACKET && packet.type != PLAYER_ANIMATION_PACKET) {
// Too many packets will get alerted if we leave this for PLAYER_UPDATE
Expand Down
38 changes: 37 additions & 1 deletion common/Packet.cpp
View file
Expand Up @@ -428,7 +428,7 @@ static void unmarshal_PLAYER_JUMPED(PacketReader& r, Packet* p) {

Packet::Packet() {
clear();
this->type = (PacketEnum) 0;
type = (PacketEnum) 0;
}

Packet::Packet(PacketEnum type) {
Expand Down Expand Up @@ -656,130 +656,166 @@ Packet::Packet(const Packet& other) {
}

Packet::~Packet() {
free();
}

void Packet::free() {
switch(type) {
case ACK_PACKET:
break;

case PLAYER_UPDATE_PACKET:
delete player_update.flags.item;
player_update.flags.item = NULL;
break;

case WEAPON_DISCHARGED_PACKET:
break;

case PLAYER_HIT_PACKET:
delete player_hit.extradata.item;
player_hit.extradata.item = NULL;
break;

case MESSAGE_PACKET:
delete message.recipient.item;
message.recipient.item = NULL;
delete message.message_text.item;
message.message_text.item = NULL;
break;

case NEW_ROUND_PACKET:
delete new_round.map_name.item;
new_round.map_name.item = NULL;
break;

case ROUND_OVER_PACKET:
break;

case SCORE_UPDATE_PACKET:
delete score_update.subject.item;
score_update.subject.item = NULL;
break;

case WELCOME_PACKET:
delete welcome.player_name.item;
welcome.player_name.item = NULL;
break;

case ANNOUNCE_PACKET:
delete announce.player_name.item;
announce.player_name.item = NULL;
break;

case GATE_UPDATE_PACKET:
break;

case JOIN_PACKET:
delete join.compat_version.item;
join.compat_version.item = NULL;
delete join.name.item;
join.name.item = NULL;
break;

case INFO_server_PACKET:
delete info_server.server_address.item;
info_server.server_address.item = NULL;
delete info_server.server_compat_version.item;
info_server.server_compat_version.item = NULL;
delete info_server.current_map_name.item;
info_server.current_map_name.item = NULL;
delete info_server.server_name.item;
info_server.server_name.item = NULL;
delete info_server.server_location.item;
info_server.server_location.item = NULL;
break;

case INFO_client_PACKET:
delete info_client.client_version.item;
info_client.client_version.item = NULL;
break;

case LEAVE_PACKET:
delete leave.message.item;
leave.message.item = NULL;
break;

case PLAYER_ANIMATION_PACKET:
delete player_animation.sprite_list.item;
player_animation.sprite_list.item = NULL;
delete player_animation.field.item;
player_animation.field.item = NULL;
break;

case REQUEST_DENIED_PACKET:
delete request_denied.message.item;
request_denied.message.item = NULL;
break;

case NAME_CHANGE_PACKET:
delete name_change.name.item;
name_change.name.item = NULL;
break;

case TEAM_CHANGE_PACKET:
break;

case REGISTER_SERVER_server_PACKET:
delete register_server_server.server_version.item;
register_server_server.server_version.item = NULL;
delete register_server_server.server_listen_address.item;
register_server_server.server_listen_address.item = NULL;
break;

case REGISTER_SERVER_metaserver_PACKET:
break;

case UNREGISTER_SERVER_PACKET:
delete unregister_server.server_listen_address.item;
unregister_server.server_listen_address.item = NULL;
break;

case UPGRADE_AVAILABLE_PACKET:
delete upgrade_available.latest_version.item;
upgrade_available.latest_version.item = NULL;
break;

case MAP_INFO_PACKET:
delete map_info.map.item;
map_info.map.item = NULL;
break;

case MAP_OBJECT_PACKET:
break;

case GAME_PARAM_PACKET:
delete game_param.param_name.item;
game_param.param_name.item = NULL;
delete game_param.param_value.item;
game_param.param_value.item = NULL;
break;

case HOLE_PUNCH_PACKET:
delete hole_punch.client_address.item;
hole_punch.client_address.item = NULL;
break;

case PLAYER_DIED_PACKET:
break;

case WEAPON_INFO_PACKET:
delete weapon_info.weapon_data.item;
weapon_info.weapon_data.item = NULL;
break;

case ROUND_START_PACKET:
break;

case SPAWN_PACKET:
delete spawn.position.item;
spawn.position.item = NULL;
delete spawn.velocity.item;
spawn.velocity.item = NULL;
break;

case PLAYER_JUMPED_PACKET:
Expand Down
1 change: 1 addition & 0 deletions common/Packet.hpp
View file
Expand Up @@ -59,6 +59,7 @@ namespace LM {
Packet(const Packet& other);
~Packet();
void clear();
void free();
void marshal();
void unmarshal();
void dispatch(PacketReceiver* r);
Expand Down
8 changes: 7 additions & 1 deletion tools/parse_idl.py
View file
Expand Up @@ -190,6 +190,7 @@ def outputHpp(interface):
code.append('\t\t{0}(const {0}& other);'.format(interface.name))
code.append('\t\t~{0}();'.format(interface.name))
code.append('\t\tvoid clear();')
code.append('\t\tvoid free();')
code.append('\t\tvoid marshal();')
code.append('\t\tvoid unmarshal();')
code.append('\t\tvoid dispatch({0}Receiver* r);'.format(interface.name, interface.name.lower()))
Expand Down Expand Up @@ -242,7 +243,7 @@ def outputCpp(interface):

code.append('{0}::{0}() {{'.format(interface.name))
code.append('\tclear();');
code.append('\tthis->type = ({0}Enum) 0;'.format(interface.name));
code.append('\ttype = ({0}Enum) 0;'.format(interface.name));
code.append('}\n')

code.append('{0}::{0}({0}Enum type) {{'.format(interface.name))
Expand All @@ -269,12 +270,17 @@ def outputCpp(interface):
code.append('}\n')

code.append('{0}::~{0}() {{'.format(interface.name))
code.append('\tfree();')
code.append('}\n')

code.append('void {0}::free() {{'.format(interface.name))
code.append('\tswitch(type) {')
for item in interface.definitions:
code.append('\tcase {1}_{0}:'.format(interface.name.upper(), item.name))
for field in item.fields:
if field.type == 'string' or field.type in interface.requirements:
code.append('\t\tdelete {0}.{1}.item;'.format(item.name.lower(), field.name))
code.append('\t\t{0}.{1}.item = NULL;'.format(item.name.lower(), field.name))
code.append('\t\tbreak;\n')
code.append('\t}')
code.append('}\n')
Expand Down

0 comments on commit 0cc6612

Please sign in to comment.