[Freeciv-Dev] Re: (PR#7216) take again client crash
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
<URL: http://rt.freeciv.org/Ticket/Display.html?id=7216 >
On Thu, Jan 08, 2004 at 09:58:24PM -0800, Mike Kaufman wrote:
>
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7216 >
>
> On Thu, Jan 08, 2004 at 09:50:26AM -0800, Raimar Falke wrote:
> >
> > <URL: http://rt.freeciv.org/Ticket/Display.html?id=7216 >
> >
> > On Thu, Jan 08, 2004 at 04:23:54AM -0800, Per I. Mathisen wrote:
> > >
> > > <URL: http://rt.freeciv.org/Ticket/Display.html?id=7216 >
> > >
> > > You crash the client (gtk2, CVS HEAD) by doing 'take' twice.
> > >
> > > To reproduce:
> > > Start savegame. Login as 'me', then 'take me player1'.
> > > Then 'take me player2'.
> > > *booom*
>
> This worked just fine pre-delta. I suggest you look for the problem there.
>
> > map.tiles is NULL at the client. It looks like packet_map_info is
> > missing somehow. However coded the take command should over it again
> > and see that map related packets (packet_map_info and
> > packet_game_state) match.
>
> I'm not sure what this means. sending the command:
> send_game_state(&aconn->self, CLIENT_PRE_GAME_STATE);
> to the client should make the client do the proper thing.
Indeed it is caused by the delta patch. Sorry for the wrong
accusation.
The packet_map_info for the second take doesn't arrive at the client
because it is redundant (at least delta things so). However the
problem is greater. Because of the new game state that the clients
gets it forgets all tile information. If you now switch back to the
first player the server however will not send the tile data again
thinking again that the client already has the information.
The reason is that you shall not delete data on one side only.
One solution is to clear the delta cache at a game state packet. The
patch implements this.
> > I also propose a patch like this or similar:
> >
> > ***************************************************************/
> > struct tile *map_get_tile(int x, int y)
> > {
> > + assert(map.tiles != NULL);
> > return MAP_TILE(x, y);
> > }
>
> absolutely not. Do you know how many times map_get_tile gets called?
It is one test on a value already loaded into a register.
Raimar
--
email: rf13@xxxxxxxxxxxxxxxxx
"How about the new language C&? No, that's not 'c ampersand', 'c reference',
'reference to c' or 'c and'. It's pronounced 'campersand', to confuse the
hell out of people who are unfamiliar with it, and it will, of course,
have no pointers."
-- Xazziri in comp.lang.c++ about C#
Index: common/connection.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/connection.c,v
retrieving revision 1.36
diff -u -u -r1.36 connection.c
--- common/connection.c 2003/12/04 19:51:21 1.36
+++ common/connection.c 2004/01/10 08:17:30
@@ -558,25 +558,15 @@
{
int i;
+ conn_clear_packet_cache(pc);
+
for (i = 0; i < PACKET_LAST; i++) {
if (pc->phs.sent[i] != NULL) {
- struct hash_table *hash = pc->phs.sent[i];
- while (hash_num_entries(hash) > 0) {
- const void *key = hash_key_by_number(hash, 0);
- hash_delete_entry(hash, key);
- free((void *) key);
- }
- hash_free(hash);
+ hash_free(pc->phs.sent[i]);
pc->phs.sent[i] = NULL;
}
if (pc->phs.received[i] != NULL) {
- struct hash_table *hash = pc->phs.received[i];
- while (hash_num_entries(hash) > 0) {
- const void *key = hash_key_by_number(hash, 0);
- hash_delete_entry(hash, key);
- free((void *) key);
- }
- hash_free(hash);
+ hash_free(pc->phs.received[i]);
pc->phs.received[i] = NULL;
}
}
@@ -628,3 +618,30 @@
free_packet_hashes(pconn);
}
+/**************************************************************************
+ Remove all cached packets from the connection. This resets the
+ delta-state.
+**************************************************************************/
+void conn_clear_packet_cache(struct connection *pc)
+{
+ int i;
+
+ for (i = 0; i < PACKET_LAST; i++) {
+ if (pc->phs.sent[i] != NULL) {
+ struct hash_table *hash = pc->phs.sent[i];
+ while (hash_num_entries(hash) > 0) {
+ const void *key = hash_key_by_number(hash, 0);
+ hash_delete_entry(hash, key);
+ free((void *) key);
+ }
+ }
+ if (pc->phs.received[i] != NULL) {
+ struct hash_table *hash = pc->phs.received[i];
+ while (hash_num_entries(hash) > 0) {
+ const void *key = hash_key_by_number(hash, 0);
+ hash_delete_entry(hash, key);
+ free((void *) key);
+ }
+ }
+ }
+}
Index: common/connection.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/connection.h,v
retrieving revision 1.32
diff -u -u -r1.32 connection.h
--- common/connection.h 2003/11/28 17:37:21 1.32
+++ common/connection.h 2004/01/10 08:17:30
@@ -250,6 +250,7 @@
void connection_common_init(struct connection *pconn);
void connection_common_close(struct connection *pconn);
void free_compression_queue(struct connection *pconn);
+void conn_clear_packet_cache(struct connection *pconn);
const char *conn_description(const struct connection *pconn);
Index: common/generate_packets.py
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/generate_packets.py,v
retrieving revision 1.5
diff -u -u -r1.5 generate_packets.py
--- common/generate_packets.py 2004/01/09 08:00:51 1.5
+++ common/generate_packets.py 2004/01/10 08:17:31
@@ -485,8 +485,9 @@
self.no=no
self.no_packet=packet.no_packet
- self.want_post=packet.want_post
- self.want_pre=packet.want_pre
+ self.want_post_recv=packet.want_post_recv
+ self.want_pre_send=packet.want_pre_send
+ self.want_post_send=packet.want_post_send
self.type=packet.type
self.delta=packet.delta
self.is_action=packet.is_action
@@ -645,7 +646,7 @@
temp='''%(send_prototype)s
{
<real_packet1><delta_header> SEND_PACKET_START(%(type)s);
-<freelog><report><pre1><body><pre2> SEND_PACKET_END;
+<freelog><report><pre1><body><pre2> <post>SEND_PACKET_END;
}
'''
@@ -660,13 +661,13 @@
freelog='\n freelog(%(log_level)s, "%(name)s: sending info about
(%(keys_format)s)"%(keys_arg)s);\n'
else:
freelog=""
- if self.want_pre:
+ if self.want_pre_send:
pre1='''
{
struct %(packet_name)s *tmp = fc_malloc(sizeof(*tmp));
*tmp = *packet;
- pre_send_%(packet_name)s(pc, %(type)s, tmp);
+ pre_send_%(packet_name)s(pc, tmp);
real_packet = tmp;
}
'''
@@ -707,6 +708,11 @@
body=""
delta_header=""
+ if self.want_post_send:
+ post=" post_send_%(packet_name)s(pc, real_packet);\n"
+ else:
+ post=""
+
for i in range(2):
for k,v in vars().items():
if type(v)==type(""):
@@ -808,8 +814,8 @@
else:
freelog=""
- if self.want_post:
- post=" post_receive_%(packet_name)s(pc, type, real_packet);\n"
+ if self.want_post_recv:
+ post=" post_receive_%(packet_name)s(pc, real_packet);\n"
else:
post=""
@@ -896,12 +902,15 @@
self.is_action="is-info" not in arr
if not self.is_action: arr.remove("is-info")
- self.want_pre="pre" in arr
- if self.want_pre: arr.remove("pre")
+ self.want_pre_send="pre-send" in arr
+ if self.want_pre_send: arr.remove("pre-send")
- self.want_post="post" in arr
- if self.want_post: arr.remove("post")
-
+ self.want_post_recv="post-recv" in arr
+ if self.want_post_recv: arr.remove("post-recv")
+
+ self.want_post_send="post-send" in arr
+ if self.want_post_send: arr.remove("post-send")
+
self.delta="no-delta" not in arr
if not self.delta: arr.remove("no-delta")
Index: common/packets.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.c,v
retrieving revision 1.265
diff -u -u -r1.265 packets.c
--- common/packets.c 2004/01/08 01:47:35 1.265
+++ common/packets.c 2004/01/10 08:17:32
@@ -597,7 +597,6 @@
}
void pre_send_packet_chat_msg(struct connection *pc,
- enum packet_type packet_type,
struct packet_chat_msg *packet)
{
if (packet->x == -1 && packet->y == -1) {
@@ -609,7 +608,6 @@
}
void post_receive_packet_chat_msg(struct connection *pc,
- enum packet_type packet_type,
struct packet_chat_msg *packet)
{
if (packet->x == 255 && packet->y == 255) {
@@ -620,7 +618,6 @@
}
void pre_send_packet_player_attribute_chunk(struct connection *pc,
- enum packet_type packet_type,
struct packet_player_attribute_chunk
*packet)
{
@@ -638,7 +635,6 @@
}
void post_receive_packet_player_attribute_chunk(struct connection *pc,
- enum packet_type packet_type,
struct
packet_player_attribute_chunk
*packet)
{
@@ -663,4 +659,15 @@
freelog(LOG_DEBUG, "received attribute chunk %d/%d %d", packet->offset,
packet->total_length, packet->chunk_length);
+}
+
+void post_receive_packet_game_state(struct connection *pc,
+ struct packet_game_state *packet)
+{
+ conn_clear_packet_cache(pc);
+}
+void post_send_packet_game_state(struct connection *pc,
+ const struct packet_game_state *packet)
+{
+ conn_clear_packet_cache(pc);
}
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.4
diff -u -u -r1.4 packets.def
--- common/packets.def 2003/12/06 20:37:59 1.4
+++ common/packets.def 2004/01/10 08:17:33
@@ -279,7 +279,7 @@
PACKET_NATION_SELECT_OK=11;sc
end
-PACKET_GAME_STATE=12;sc
+PACKET_GAME_STATE=12;post-send,post-recv,sc
UINT32 value;
end
@@ -356,7 +356,7 @@
/************** Chat/event packets **********************/
-PACKET_CHAT_MSG=18;sc, pre, post
+PACKET_CHAT_MSG=18;sc, pre-send, post-recv
STRING message[MAX_LEN_MSG];
COORD x, y;
EVENT event;
@@ -574,7 +574,7 @@
PACKET_PLAYER_ATTRIBUTE_BLOCK=46;cs
end
-PACKET_PLAYER_ATTRIBUTE_CHUNK=47; pre,post,sc,cs,handle-via-packet
+PACKET_PLAYER_ATTRIBUTE_CHUNK=47; pre-send,post-recv,sc,cs,handle-via-packet
UINT32 offset, total_length, chunk_length;
/* to keep memory management simple don't allocate dynamic memory */
MEMORY data[ATTRIBUTE_CHUNK_SIZE:chunk_length];
Index: common/packets.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.h,v
retrieving revision 1.161
diff -u -u -r1.161 packets.h
--- common/packets.h 2003/12/01 18:18:01 1.161
+++ common/packets.h 2004/01/10 08:17:33
@@ -79,19 +79,19 @@
const char *get_packet_name(enum packet_type type);
void pre_send_packet_chat_msg(struct connection *pc,
- enum packet_type packet_type,
struct packet_chat_msg *packet);
void post_receive_packet_chat_msg(struct connection *pc,
- enum packet_type packet_type,
struct packet_chat_msg *packet);
void pre_send_packet_player_attribute_chunk(struct connection *pc,
- enum packet_type packet_type,
struct packet_player_attribute_chunk
*packet);
-void post_receive_packet_player_attribute_chunk(struct connection *pc,
- enum packet_type packet_type,
- struct
packet_player_attribute_chunk
+void post_receive_packet_player_attribute_chunk(struct connection *pc, struct
packet_player_attribute_chunk
*packet);
+
+void post_receive_packet_game_state(struct connection *pc,
+ struct packet_game_state *packet);
+void post_send_packet_game_state(struct connection *pc,
+ const struct packet_game_state *packet);
#define SEND_PACKET_START(type) \
unsigned char buffer[MAX_LEN_PACKET]; \
Index: common/packets_gen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets_gen.c,v
retrieving revision 1.5
diff -u -u -r1.5 packets_gen.c
--- common/packets_gen.c 2004/01/09 08:00:51 1.5
+++ common/packets_gen.c 2004/01/10 08:17:37
@@ -2042,6 +2042,7 @@
}
hash_insert(*hash, clone, clone);
+ post_receive_packet_game_state(pc, real_packet);
RECEIVE_PACKET_END(real_packet);
}
@@ -2091,7 +2092,8 @@
*clone = *real_packet;
hash_insert(*hash, clone, clone);
- SEND_PACKET_END;
+ post_send_packet_game_state(pc, real_packet);
+SEND_PACKET_END;
}
static void ensure_valid_variant_packet_game_state(struct connection *pc)
@@ -3967,7 +3969,7 @@
}
hash_insert(*hash, clone, clone);
- post_receive_packet_chat_msg(pc, type, real_packet);
+ post_receive_packet_chat_msg(pc, real_packet);
RECEIVE_PACKET_END(real_packet);
}
@@ -3985,7 +3987,7 @@
struct packet_chat_msg *tmp = fc_malloc(sizeof(*tmp));
*tmp = *packet;
- pre_send_packet_chat_msg(pc, PACKET_CHAT_MSG, tmp);
+ pre_send_packet_chat_msg(pc, tmp);
real_packet = tmp;
}
@@ -9417,7 +9419,7 @@
}
hash_insert(*hash, clone, clone);
- post_receive_packet_player_attribute_chunk(pc, type, real_packet);
+ post_receive_packet_player_attribute_chunk(pc, real_packet);
RECEIVE_PACKET_END(real_packet);
}
@@ -9435,7 +9437,7 @@
struct packet_player_attribute_chunk *tmp = fc_malloc(sizeof(*tmp));
*tmp = *packet;
- pre_send_packet_player_attribute_chunk(pc, PACKET_PLAYER_ATTRIBUTE_CHUNK,
tmp);
+ pre_send_packet_player_attribute_chunk(pc, tmp);
real_packet = tmp;
}
|
|
