View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001931 | FSSCP | user interface | public | 2009-05-30 00:22 | 2022-06-11 00:59 |
| Reporter | FUBAR-BDHR | Assigned To | |||
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.9 | ||||
| Fixed in Version | 22.2.0 | ||||
| Summary | 0001931: Having more than 39 ship classes in loadout causes crash | ||||
| Description | TBP now has 40 player allowed ships and one multiplayer mission was set up for all 40. No problems adding them in FRED but every time you try to select a ship in loadout you crash. 39 ships = no problem and it doesn't matter which ship I remove. Crash is at line 1700 in missionscreencommon.cpp | ||||
| Additional Information | 3.6.10 RC2 running Ultimate Gauntlet. Bad version is in Zathras RC3. I fixed it by removing the maint bot so if you are using a newer version just allow it in FRED. | ||||
| Tags | No tags attached. | ||||
|
|
Client or server side crash? (Or client selection causing a server side crash?). At first glance I suspect that this is either due to the number of ship or weapon classes in TBP rather than an odd limit of 39 ships. What happens if you leave the mainbot in and take out the Starfury then put it back? |
|
|
Host definitely. Didn't try client myself but Andicirk said clients crashed too either at loadout or mission start. Guessing they crash at mission start if they don't try to change ships. |
|
|
Works fine for me with the 3.6.11 build. This could be due to something that was fixed when I edited things for team loadout. |
|
|
Still getting it. Just compiled 3.6.11 inferno. It actually gave me some debug info. Run-time check failure 0000002 - stack around the variable 'wss_data' was corrupted. And from the log: ASSERTION: "offset < max_size" at missionscreencommon.cpp:1718 Actually getting it even with the 39 ship version that worked in 3.6.10 RC3 More testing 38 now seems to be the limit. If it still works for you can you get the 3.6.11 Inferno for me to test with. |
|
2009-06-03 03:46
|
|
|
|
Works just fine for me. This may be due to a packet being sent from client to server (I was trying a multiplayer coop on my own to test.) I'll try with a client present later today. |
|
|
Based on the debug info FUBAR provided, I'd say this crash is caused by the memcpy() at missionscreencommon.cpp:1715. memcpy(block+offset,&player_id,sizeof(player_id)); The 'offset' is used internally in store_wss_data() to keep track of how far past the start of the 'block' passed into store_wss_data() we should be writing, but it seems there's no check done before each memcpy that 'offset' isn't already past, or will go past, the 'max_size' passed in. While we would still have to trust that the 'max_size' passed is correct, there's at least one spot in store_wss_data() where the Assert() should go before the memory operation. This attached patch won't solve the underlying cause - the inputs to store_wss_data() can't accommodate all the data already and about to be written to the block - but it should stop the memory corruption, with an Assert() instead. |
|
2009-06-03 11:38
|
mantis-1931.diff (437 bytes)
Index: code/missionui/missionscreencommon.cpp =================================================================== --- code/missionui/missionscreencommon.cpp (revision 5327) +++ code/missionui/missionscreencommon.cpp (working copy) @@ -1712,6 +1712,7 @@ } player_id = INTEL_SHORT( player_id ); + Assert( offset += sizeof(player_id) < max_size ); memcpy(block+offset,&player_id,sizeof(player_id)); offset += sizeof(player_id); |
|
|
The problem is that the Assert only solves one problem while leaving a whole load more open and waiting to bite us on the arse. And it doesn't do anything to solve the problem in release builds. Since the offset is also increased in the loops for ships and weapons there is a chance that it could go over the packet size limit in them too. Admittedly it's unlikely to do so in ship unless we bump the number of ships allowed again but it's very likely to happen in the weapons loop. Since ships add a ubyte while weapons add a short a single extra weapon would have corrupted the mission in a different place. One solution I can see to the problem is to break the update into two packets (Like I did with the ship update packet that used to break Inferno multiplayer). Another would simply be to bump MAX_PACKET_SIZE. Although I'm not certain what else that could break. Either way, whatever we do here is going to break compatibility with older builds unless we simply make it error out. |
|
|
Any updates on the network packet change? |
|
|
I think I'm going to add this to my proposed rewrite of the loadout code. There are just too many things wrong with it to waste time patching it now when the entire thing really needs to be rewritten without the assumptions that Volition made when they originally wrote the code. |
|
|
I think this this thread probably hit the same issue? http://www.hard-light.net/forums/index.php?topic=88136.0 I had a look in TBP's ship tables to see if there were any other fighter-class ships that I could add to the mission FUBAR_MG3.fs2, but I couldn't see anything obvious. I'll instead have a go at reproducing it in the Aerotech TC. |
|
|
Since Karajorma wants to rewrite the loadout code, this shouldn't be targeted for 3.7.2. Karajorma, is there any sort of error condition you can add in the meantime to have the builds fail gracefully, rather than crashing? |
|
|
I can definitely look into a better solution than a crash. |
|
|
Close as this section of code was redone during the multi packet fixes. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2009-05-30 00:22 | FUBAR-BDHR | New Issue | |
| 2009-05-30 07:25 | karajorma | Note Added: 0010921 | |
| 2009-05-30 07:26 | karajorma | Status | new => assigned |
| 2009-05-30 07:26 | karajorma | Assigned To | => karajorma |
| 2009-05-30 07:34 | FUBAR-BDHR | Note Added: 0010922 | |
| 2009-06-02 21:13 | karajorma | Note Added: 0010934 | |
| 2009-06-03 03:24 | FUBAR-BDHR | Note Added: 0010935 | |
| 2009-06-03 03:44 | FUBAR-BDHR | Note Edited: 0010935 | |
| 2009-06-03 03:46 | FUBAR-BDHR | File Added: FUBAR_MG03.FS2 | |
| 2009-06-03 05:00 | FUBAR-BDHR | Note Edited: 0010935 | |
| 2009-06-03 10:06 | karajorma | Note Added: 0010936 | |
| 2009-06-03 11:38 | Echelon9 | Note Added: 0010937 | |
| 2009-06-03 11:38 | Echelon9 | File Added: mantis-1931.diff | |
| 2009-06-03 11:41 | Echelon9 | Note Edited: 0010937 | |
| 2009-06-03 11:41 | Echelon9 | Note Edited: 0010937 | |
| 2009-06-03 20:04 | karajorma | Note Added: 0010939 | |
| 2012-04-03 13:42 | Echelon9 | Note Added: 0013448 | |
| 2012-04-03 13:42 | Echelon9 | Status | assigned => feedback |
| 2012-12-30 07:09 | karajorma | Note Added: 0014588 | |
| 2012-12-30 07:09 | karajorma | Target Version | => 3.7.2 |
| 2012-12-30 07:10 | karajorma | Assigned To | karajorma => |
| 2014-08-26 11:16 | niffiwan | Note Added: 0016262 | |
| 2014-09-24 06:40 | Goober5000 | Note Added: 0016293 | |
| 2014-09-24 06:40 | Goober5000 | Target Version | 3.7.2 => |
| 2014-10-08 02:06 | karajorma | Note Added: 0016330 | |
| 2022-06-10 20:58 | FSCyborg | Note Added: 0017134 | |
| 2022-06-10 20:58 | FSCyborg | Status | feedback => closed |
| 2022-06-11 00:59 | Goober5000 | Status | closed => resolved |
| 2022-06-11 00:59 | Goober5000 | Resolution | open => fixed |
| 2022-06-11 00:59 | Goober5000 | Fixed in Version | => 22.2.0 |
| 2022-06-11 00:59 | Goober5000 | Summary | Having more then 39 ship classes in loadout casues crash => Having more than 39 ship classes in loadout causes crash |
