From 8c56f1aa0b4a52fb824daf069e3893068e130901 Mon Sep 17 00:00:00 2001 From: fickleheart Date: Mon, 25 Mar 2019 22:22:25 -0500 Subject: [PATCH 01/12] Wacky attempt at not reopening nodes for D/Ced ackreting clients --- src/d_net.c | 13 +++++++++---- src/i_tcp.c | 11 +++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/d_net.c b/src/d_net.c index 9f719967..22ee738f 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -1146,7 +1146,7 @@ boolean HSendPacket(INT32 node, boolean reliable, UINT8 acknum, size_t packetlen // boolean HGetPacket(void) { - //boolean nodejustjoined; + boolean nodejustjoined; // Get a packet from self if (rebound_tail != rebound_head) @@ -1173,11 +1173,16 @@ boolean HGetPacket(void) while(true) { - //nodejustjoined = I_NetGet(); - I_NetGet(); + nodejustjoined = I_NetGet(); + //I_NetGet(); if (doomcom->remotenode == -1) // No packet received - return false; + { + if (nodejustjoined) // _This_ means we did receive a packet, but either from a node we couldn't allocate or a gone player ackreting... + continue; + else + return false; + } getbytes += packetheaderlength + doomcom->datalength; // For stat diff --git a/src/i_tcp.c b/src/i_tcp.c index 11a84ceb..0c7f7cc9 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -624,6 +624,13 @@ static boolean SOCK_Get(void) } // not found + if (netbuffer->packettype == PT_NOTHING) + { + DEBFILE(va("Ackret received from disconnected address:%s, ignoring...\n", SOCK_AddrToStr(&fromaddress))); + doomcom->remotenode = -1; // no packet + return true; + } + // find a free slot j = getfreenode(); if (j > 0) @@ -650,7 +657,11 @@ static boolean SOCK_Get(void) return true; } else + { DEBFILE("New node detected: No more free slots\n"); + doomcom->remotenode = -1; // no packet + return true; + } } } From a35e9d5abdf1e6ad97aa104308572a60795f815c Mon Sep 17 00:00:00 2001 From: fickleheart Date: Mon, 25 Mar 2019 22:23:35 -0500 Subject: [PATCH 02/12] Don't force-close connection when getting a join request! We need to send the ackret, first of all! --- src/d_clisrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index e227ce2e..ba0446c0 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -3980,7 +3980,7 @@ static void HandlePacketFromAwayNode(SINT8 node) cl_challengenum = netbuffer->u.joinchallenge.challengenum; memcpy(cl_challengequestion, netbuffer->u.joinchallenge.question, 16); - Net_CloseConnection(node|FORCECLOSE); // Don't need to stay connected while challenging + Net_CloseConnection(node); // Don't need to stay connected while challenging cl_mode = CL_CHALLENGE; From 34557910543b60148f7bcbdbc13083ef5d161aee Mon Sep 17 00:00:00 2001 From: fickleheart Date: Mon, 25 Mar 2019 22:44:34 -0500 Subject: [PATCH 03/12] More attempts that may fix things? I'm uncommenting a bunch of LJ code that was never committed uncommented, so I might be breaking something horribly here.... --- src/d_net.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/d_net.c b/src/d_net.c index 22ee738f..164be502 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -1184,6 +1184,13 @@ boolean HGetPacket(void) return false; } + if (nodejustjoined) + { + // Reinitialize vars for the new node just in case there's anything left over from other players..... + InitNode(&nodes[doomcom->remotenode]); + SV_AbortSendFiles(doomcom->remotenode); + } + getbytes += packetheaderlength + doomcom->datalength; // For stat if (doomcom->remotenode >= MAXNETNODES) @@ -1197,8 +1204,8 @@ boolean HGetPacket(void) if (netbuffer->checksum != NetbufferChecksum()) { DEBFILE("Bad packet checksum\n"); - //Net_CloseConnection(nodejustjoined ? (doomcom->remotenode | FORCECLOSE) : doomcom->remotenode); - Net_CloseConnection(doomcom->remotenode); + Net_CloseConnection(nodejustjoined ? (doomcom->remotenode | FORCECLOSE) : doomcom->remotenode); + //Net_CloseConnection(doomcom->remotenode); continue; } @@ -1207,7 +1214,7 @@ boolean HGetPacket(void) DebugPrintpacket("GET"); #endif - /*// If a new node sends an unexpected packet, just ignore it + // If a new node sends an unexpected packet, just ignore it if (nodejustjoined && server && !(netbuffer->packettype == PT_ASKINFO || netbuffer->packettype == PT_SERVERINFO @@ -1220,7 +1227,7 @@ boolean HGetPacket(void) //CONS_Alert(CONS_NOTICE, "New node sent an unexpected %s packet\n", packettypename[netbuffer->packettype]); Net_CloseConnection(doomcom->remotenode | FORCECLOSE); continue; - }*/ + } // Proceed the ack and ackreturn field if (!Processackpak()) From 0485b4cf5c922f7dd85ef96fba3a613b894c1a12 Mon Sep 17 00:00:00 2001 From: fickleheart Date: Mon, 25 Mar 2019 22:49:37 -0500 Subject: [PATCH 04/12] Make sure clients never have local_resynchinprogress on when joining a game... --- src/d_clisrv.c | 2 ++ src/d_net.c | 1 + 2 files changed, 3 insertions(+) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index ba0446c0..811645da 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2304,6 +2304,8 @@ static void CL_ConnectToServer(boolean viams) if (gamestate == GS_VOTING) Y_EndVote(); + resynch_local_inprogress = false; // Just in case this was never cleared... + DEBFILE(va("waiting %d nodes\n", doomcom->numnodes)); G_SetGamestate(GS_WAITINGPLAYERS); wipegamestate = GS_WAITINGPLAYERS; diff --git a/src/d_net.c b/src/d_net.c index 164be502..0219f24d 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -1474,6 +1474,7 @@ void D_CloseConnection(void) I_NetMakeNodewPort = NULL; netgame = false; addedtogame = false; + resynch_local_inprogress = false; // No more resyncing! } D_ResetTiccmds(); From 29f460a21f7c829259514044801f841614fc9c7a Mon Sep 17 00:00:00 2001 From: fickleheart Date: Mon, 25 Mar 2019 22:52:42 -0500 Subject: [PATCH 05/12] pbbbbt fix compile error --- src/d_clisrv.c | 1 + src/d_net.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 811645da..9bd33c4a 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -3413,6 +3413,7 @@ void D_QuitNetGame(void) HSendPacket(servernode, true, 0, 0); } + resynch_local_inprogress = false; // No more resyncing! D_CloseConnection(); ClearAdminPlayers(); diff --git a/src/d_net.c b/src/d_net.c index 0219f24d..164be502 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -1474,7 +1474,6 @@ void D_CloseConnection(void) I_NetMakeNodewPort = NULL; netgame = false; addedtogame = false; - resynch_local_inprogress = false; // No more resyncing! } D_ResetTiccmds(); From 204a6d5c202c29203027223c7c0fc2423d97265f Mon Sep 17 00:00:00 2001 From: fickleheart Date: Mon, 25 Mar 2019 23:58:44 -0500 Subject: [PATCH 06/12] Let's try some sanity checks on the resync code... --- src/d_clisrv.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 9bd33c4a..0b7e465f 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -1052,6 +1052,7 @@ static void SV_SendResynch(INT32 node) return; } + resynch_inprogress[node] = false; // Let's see if there's REALLY anyone left to sync..... netbuffer->packettype = PT_RESYNCHING; for (i = 0, j = 0; i < MAXPLAYERS; ++i) { @@ -1059,10 +1060,24 @@ static void SV_SendResynch(INT32 node) if (!(resynch_status[node] & 1< TICRATE) + { + CONS_Alert(CONS_ERROR, "Node %d (%s) somehow had a stupidly-long resync delay?! (%d tics to resync player %d)\n", + node, player_names[nodetoplayer[node]], resynch_sent[node][i], i + ); + resynch_sent[node][i] = TICRATE; + } + continue; } @@ -1076,6 +1091,15 @@ static void SV_SendResynch(INT32 node) break; } + if (!resynch_inprogress[node]) + { + CONS_Alert(CONS_ERROR, "Node %d (%s) somehow had resync status for nonexistent players?! (%08x)\n", + node, player_names[nodetoplayer[node]], resynch_status[node] = 0x00 + ); + resynch_status[node] = 0x00; + resynch_inprogress[node] = true; // So they get the PT_RESYNCHEND... + } + if (resynch_score[node] > (unsigned)cv_resynchattempts.value*250) { XBOXSTATIC UINT8 buf[2]; From 6cf7cc0276cb20d8730f92678bd60da1f93d34eb Mon Sep 17 00:00:00 2001 From: fickleheart Date: Tue, 26 Mar 2019 00:09:23 -0500 Subject: [PATCH 07/12] Is this useful in any way? --- src/d_clisrv.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 0b7e465f..31f73d78 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -688,7 +688,7 @@ static inline void resynch_write_player(resynch_pak *rsp, const size_t i) static void resynch_read_player(resynch_pak *rsp) { INT32 i = rsp->playernum, j; - mobj_t *savedmo = players[i].mo; + //mobj_t *savedmo = players[i].mo; // Do not send anything visual related. // Only send data that we need to know for physics. @@ -795,11 +795,17 @@ static void resynch_read_player(resynch_pak *rsp) return; //...but keep old mo even if it is corrupt or null! - players[i].mo = savedmo; + //players[i].mo = savedmo; //Transfer important mo information if they have a valid mo. if (!rsp->hasmo) + { + // Get rid of their object if they aren't supposed to have one.....?? + if (players[i].mo) + P_RemoveMobj(players[i].mo); + return; + } //server thinks player has a body. //Give them a new body that can be then manipulated by the server's info. From ca2979c1af0613a857a8d4c2b86d06e7d8f1ba2a Mon Sep 17 00:00:00 2001 From: fickleheart Date: Tue, 26 Mar 2019 01:10:00 -0500 Subject: [PATCH 08/12] Free nodes properly in cleanupnodes Hopefully the other changes herein have rendered cleanupnodes obsolete, but I'm not too sure of that..... --- src/i_tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/i_tcp.c b/src/i_tcp.c index 0c7f7cc9..ab14cdc9 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -524,7 +524,7 @@ static void cleanupnodes(void) // Why can't I start at zero? for (j = 1; j < MAXNETNODES; j++) if (!(nodeingame[j] || SV_SendingFile(j))) - nodeconnected[j] = false; + SOCK_FreeNodenum(j); // At least free this PROPERLY } static SINT8 getfreenode(void) From 7090cf16b408424734e6a7f075fa898088420a39 Mon Sep 17 00:00:00 2001 From: fickleheart Date: Tue, 26 Mar 2019 01:36:12 -0500 Subject: [PATCH 09/12] Unclose a node if it sends PT_CLIENTJOIN..? fuck it, why not --- src/d_clisrv.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 31f73d78..7ed98e40 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -1374,6 +1374,8 @@ static boolean CL_SendJoin(void) netbuffer->u.clientcfg.challengenum = cl_challengenum; memcpy(netbuffer->u.clientcfg.challengeanswer, cl_challengeanswer, MD5_LEN); + nodes[servernode].flags &= ~NF_CLOSE; // Hell if I know. + return HSendPacket(servernode, true, 0, sizeof (clientconfig_pak)); } @@ -3830,6 +3832,9 @@ static void HandleConnect(SINT8 node) } } + // The connecting node may be + nodes[node].flags &= ~NF_CLOSE; + if (netbuffer->u.clientcfg.needsdownload) { netbuffer->packettype = PT_DOWNLOADFILESOKAY; From 9bc3acd17a56aa0a3e678773a7fd996909a0d103 Mon Sep 17 00:00:00 2001 From: fickleheart Date: Tue, 26 Mar 2019 08:26:41 -0500 Subject: [PATCH 10/12] Fix compile issues --- src/d_clisrv.c | 5 ----- src/d_net.c | 8 ++++++++ src/i_tcp.c | 2 ++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 7ed98e40..31f73d78 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -1374,8 +1374,6 @@ static boolean CL_SendJoin(void) netbuffer->u.clientcfg.challengenum = cl_challengenum; memcpy(netbuffer->u.clientcfg.challengeanswer, cl_challengeanswer, MD5_LEN); - nodes[servernode].flags &= ~NF_CLOSE; // Hell if I know. - return HSendPacket(servernode, true, 0, sizeof (clientconfig_pak)); } @@ -3832,9 +3830,6 @@ static void HandleConnect(SINT8 node) } } - // The connecting node may be - nodes[node].flags &= ~NF_CLOSE; - if (netbuffer->u.clientcfg.needsdownload) { netbuffer->packettype = PT_DOWNLOADFILESOKAY; diff --git a/src/d_net.c b/src/d_net.c index 164be502..954e7529 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -1110,6 +1110,10 @@ boolean HSendPacket(INT32 node, boolean reliable, UINT8 acknum, size_t packetlen netbuffer->checksum = NetbufferChecksum(); sendbytes += packetheaderlength + doomcom->datalength; // For stat + // Joinpasswords close nodes, this may try to send to a waiting-to-close node, so cancel closing? + if (netbuffer->packettype == PT_CLIENTJOIN) + nodes[node].flags &= ~NF_CLOSE; + #ifdef PACKETDROP // Simulate internet :) //if (rand() >= (INT32)(RAND_MAX * (PACKETLOSSRATE / 100.f))) @@ -1191,6 +1195,10 @@ boolean HGetPacket(void) SV_AbortSendFiles(doomcom->remotenode); } + // Joinpasswords close nodes, this may receive from a waiting-to-close node, so cancel closing? + if (netbuffer->packettype == PT_CLIENTJOIN) + nodes[doomcom->remotenode].flags &= ~NF_CLOSE; + getbytes += packetheaderlength + doomcom->datalength; // For stat if (doomcom->remotenode >= MAXNETNODES) diff --git a/src/i_tcp.c b/src/i_tcp.c index ab14cdc9..4989fbd2 100644 --- a/src/i_tcp.c +++ b/src/i_tcp.c @@ -508,6 +508,8 @@ static boolean SOCK_cmpaddr(mysockaddr_t *a, mysockaddr_t *b, UINT8 mask) return false; } +static void SOCK_FreeNodenum(INT32 numnode); + // This is a hack. For some reason, nodes aren't being freed properly. // This goes through and cleans up what nodes were supposed to be freed. /** \warning This function causes the file downloading to stop if someone joins. From 0911ec79d15be27cfa2fd3d2ea5bde600d26896e Mon Sep 17 00:00:00 2001 From: fickleheart Date: Sat, 30 Mar 2019 22:48:55 -0500 Subject: [PATCH 11/12] Try to fix hangs caused by PT_CLIENTJOIN with a non-zero ack --- src/d_net.c | 49 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/d_net.c b/src/d_net.c index 954e7529..acbd34be 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -363,7 +363,11 @@ static boolean Processackpak(void) { UINT8 ack = netbuffer->ack; getackpacket++; - if (cmpack(ack, node->firstacktosend) <= 0) + + if (cmpack(ack, node->firstacktosend) <= 0 + // UGLY PROBABLY-BAD HACK: If we get PT_CLIENTJOIN, assume this is an in-order packet? + && netbuffer->packettype != PT_CLIENTJOIN + ) { DEBFILE(va("Discard(1) ack %d (duplicated)\n", ack)); duppacket++; @@ -385,6 +389,11 @@ static boolean Processackpak(void) // Is a good packet so increment the acknowledge number, // Then search for a "hole" in the queue UINT8 nextfirstack = (UINT8)(node->firstacktosend + 1); + + // UGLY PROBABLY-BAD HACK: If we get PT_CLIENTJOIN, assume this is an in-order packet? + if (netbuffer->packettype == PT_CLIENTJOIN) + node->firstacktosend = (UINT8)((ack-1+MAXACKTOSEND) % MAXACKTOSEND); + if (!nextfirstack) nextfirstack = 1; @@ -1190,6 +1199,29 @@ boolean HGetPacket(void) if (nodejustjoined) { + // If a new node sends an unexpected packet, just ignore it + if (server + && !(netbuffer->packettype == PT_ASKINFO + || netbuffer->packettype == PT_SERVERINFO + || netbuffer->packettype == PT_PLAYERINFO + || netbuffer->packettype == PT_REQUESTFILE + || netbuffer->packettype == PT_ASKINFOVIAMS + || netbuffer->packettype == PT_CLIENTJOIN)) + { + DEBFILE(va("New node sent an unexpected %s packet\n", packettypename[netbuffer->packettype])); + CONS_Alert(CONS_NOTICE, "New node sent an unexpected %s packet\n", packettypename[netbuffer->packettype]); + Net_CloseConnection(doomcom->remotenode | FORCECLOSE); + continue; + } + + if (netbuffer->ack > 1 && !(server && netbuffer->packettype == PT_CLIENTJOIN)) + { + DEBFILE("New node sent a packet with an out-of-sequence ack. Ghost connection? Ignoring...\n"); + CONS_Alert(CONS_NOTICE, "New node sent a packet with an out-of-sequence ack. Ghost connection? Ignoring...\n"); + Net_CloseConnection(doomcom->remotenode | FORCECLOSE); + continue; + } + // Reinitialize vars for the new node just in case there's anything left over from other players..... InitNode(&nodes[doomcom->remotenode]); SV_AbortSendFiles(doomcom->remotenode); @@ -1222,21 +1254,6 @@ boolean HGetPacket(void) DebugPrintpacket("GET"); #endif - // If a new node sends an unexpected packet, just ignore it - if (nodejustjoined && server - && !(netbuffer->packettype == PT_ASKINFO - || netbuffer->packettype == PT_SERVERINFO - || netbuffer->packettype == PT_PLAYERINFO - || netbuffer->packettype == PT_REQUESTFILE - || netbuffer->packettype == PT_ASKINFOVIAMS - || netbuffer->packettype == PT_CLIENTJOIN)) - { - DEBFILE(va("New node sent an unexpected %s packet\n", packettypename[netbuffer->packettype])); - //CONS_Alert(CONS_NOTICE, "New node sent an unexpected %s packet\n", packettypename[netbuffer->packettype]); - Net_CloseConnection(doomcom->remotenode | FORCECLOSE); - continue; - } - // Proceed the ack and ackreturn field if (!Processackpak()) continue; // discarded (duplicated) From b434eabf56a3ee03a6f58b6d8fc2ff478ab4470e Mon Sep 17 00:00:00 2001 From: fickleheart Date: Sat, 30 Mar 2019 23:15:11 -0500 Subject: [PATCH 12/12] Alternate variant of ugly probably-bad hack --- src/d_net.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/d_net.c b/src/d_net.c index acbd34be..2562de9a 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -364,10 +364,7 @@ static boolean Processackpak(void) UINT8 ack = netbuffer->ack; getackpacket++; - if (cmpack(ack, node->firstacktosend) <= 0 - // UGLY PROBABLY-BAD HACK: If we get PT_CLIENTJOIN, assume this is an in-order packet? - && netbuffer->packettype != PT_CLIENTJOIN - ) + if (cmpack(ack, node->firstacktosend) <= 0) { DEBFILE(va("Discard(1) ack %d (duplicated)\n", ack)); duppacket++; @@ -390,10 +387,6 @@ static boolean Processackpak(void) // Then search for a "hole" in the queue UINT8 nextfirstack = (UINT8)(node->firstacktosend + 1); - // UGLY PROBABLY-BAD HACK: If we get PT_CLIENTJOIN, assume this is an in-order packet? - if (netbuffer->packettype == PT_CLIENTJOIN) - node->firstacktosend = (UINT8)((ack-1+MAXACKTOSEND) % MAXACKTOSEND); - if (!nextfirstack) nextfirstack = 1; @@ -1214,6 +1207,10 @@ boolean HGetPacket(void) continue; } + // UGLY PROBABLY-BAD HACK: If we get PT_CLIENTJOIN, assume this is an in-order packet? + if (netbuffer->packettype == PT_CLIENTJOIN) + nodes[doomcom->remotenode].firstacktosend = (UINT8)((netbuffer->ack-1+MAXACKTOSEND) % MAXACKTOSEND); + if (netbuffer->ack > 1 && !(server && netbuffer->packettype == PT_CLIENTJOIN)) { DEBFILE("New node sent a packet with an out-of-sequence ack. Ghost connection? Ignoring...\n");