View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000415 | FSSCP | gameplay | public | 2005-05-12 09:14 | 2005-05-18 12:58 |
| Reporter | Goober5000 | Assigned To | taylor | ||
| Priority | urgent | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Summary | 0000415: CTD upon exit | ||||
| Description | In the most recent CVS, the game CTDs whenever it exits. I think Phreak said this had something to do with shoddy memory management. Needs fixing ASAP. | ||||
| Tags | No tags attached. | ||||
|
|
it has to do with the dynamically allocated medals stuff WMC implemented. passing the buck. |
|
|
Specifically this is a problem with the promotion_text getting free'd. Happens every time for me with the "Double Ace" text specifically. Only the Triple Ace promotion text is actually correct, the other two are either trash or get overwritten by something else. EDIT: Here is a backtrace from the Linux version... [New Thread 1084229984 (LWP 19213)] [New Thread 1094719840 (LWP 19214)] [Thread 1084229984 (zombie) exited] [Thread 1094719840 (LWP 19214) exited] *** glibc detected *** double free or corruption (out): 0x0000000003116650 *** Program received signal SIGABRT, Aborted. [Switching to Thread 46912498041280 (LWP 19212)] 0x0000003e5522e37d in *__GI_raise (sig=19212) at ../nptl/sysdeps/unix/sysv/linux/raise.c:67 67 int res = INLINE_SYSCALL (tgkill, 3, pid, selftid, sig); (gdb) bt #0 0x0000003e5522e37d in *__GI_raise (sig=19212) at ../nptl/sysdeps/unix/sysv/linux/raise.c:67 0000001 0x0000003e5522faae in *__GI_abort () at ../sysdeps/generic/abort.c:88 0000002 0x0000003e55262a01 in __libc_message (do_abort=2, fmt=0x3e55310830 "*** glibc detected *** %s: 0x%s ***\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:145 0000003 0x0000003e552685ce in _int_free (av=0x3e5542f620, mem=0x4b0c) at malloc.c:5523 0000004 0x0000003e55268916 in __libc_free (mem=0x6) at malloc.c:3404 0000005 0x00000000006d8459 in _vm_free (ptr=0x3116650, filename=0x76cd63 "./stats/medals.h", line=110) at windows_stub/stubs.cpp:589 0000006 0x00000000006951aa in ~medal_stuff (this=0x311f418) at medals.h:110 0000007 0x0000000000695c33 in std::_Destroy<medal_stuff> (__pointer=0x311f418) at stl_construct.h:107 0000008 0x0000000000695fb5 in std::__destroy_aux<medal_stuff*> (__first=0x311f418, __last=0x311f580) at stl_construct.h:120 0000009 0x0000000000695f8f in std::_Destroy<medal_stuff*> (__first=0x311ed10, __last=0x311f580) at stl_construct.h:152 0000010 0x0000000000695e99 in ~vector (this=0x2db7090) at stl_vector.h:256 #11 0x0000000000695164 in __tcf_0 () at stats/medals.cpp:218 0000012 0x0000003e55230cd5 in *__GI_exit (status=0) at exit.c:60 0000013 0x0000003e5521c4c2 in __libc_start_main (main=0x413eaa <main>, argc=4, ubp_av=0x7ffffffff808, init=0x710640 <__libc_csu_init>, fini=0, rtld_fini=0x4b0c, stack_end=0x7ffffffff7f8) at ../sysdeps/generic/libc-start.c:240 0000014 0x000000000040801a in _start () 0000015 0x00007ffffffff7f8 in ?? () 0000016 0x000000000000001c in ?? () 0000017 0x0000000000000004 in ?? () 0000018 0x00007ffffffffa50 in ?? () 0000019 0x00007ffffffffa77 in ?? () 0000020 0x00007ffffffffa7c in ?? () 0000021 0x00007ffffffffa80 in ?? () 0000022 0x0000000000000000 in ?? () edited on: 05-12-05 18:28 |
|
|
Yeah I was going to guess it was something like that. Probably one of the pointers isn't getting set to NULL. Edit: Er, ISN'T edited on: 05-13-05 02:22 |
|
|
here's the data: - this 0x0325c868 + name 0x0325c868 "Ace" + bitmap 0x0325c888 "Medal14.pcx" num_versions 1 kills_needed 60 i 0 + voice_base 0x0325c8b4 "badge_a.wav" + promotion_text 0x084ed760 "îþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþ" This may have something to do with the vector resizing itself. If i declare the vector with an inital size of 20, then the crash doesn't occur. edited on: 05-13-05 15:47 |
|
2005-05-14 12:08
|
fs2-medals.diff (2,091 bytes)
--- code/stats/medals.orig Sat May 14 12:41:06 2005
+++ code/stats/medals.cpp Sat May 14 14:57:33 2005
@@ -375,13 +375,11 @@
reset_parse();
// parse in all the rank names
- medal_stuff temp_medal;
Num_medals = 0;
required_string("#Medals");
while ( required_string_either("#End", "$Name:") )
{
- //Clear this
- temp_medal = medal_stuff();
+ medal_stuff temp_medal;
required_string("$Name:");
stuff_string( temp_medal.name, F_NAME, NULL );
@@ -819,3 +817,24 @@
gr_bitmap(Medal_coords[gr_screen.res][RANK_MEDAL_REGION][0], Medal_coords[gr_screen.res][RANK_MEDAL_REGION][1]);
}
+void medal_stuff::clone(const medal_stuff &m) {
+ memcpy(name, m.name, NAME_LENGTH);
+ memcpy(bitmap, m.bitmap, NAME_LENGTH);
+ num_versions = m.num_versions;
+ kills_needed = m.kills_needed;
+ memcpy(voice_base, m.voice_base, MAX_FILENAME_LEN);
+ if (m.promotion_text)
+ promotion_text = vm_strdup(m.promotion_text);
+ else
+ promotion_text = NULL;
+}
+
+// assignment operator
+const medal_stuff &medal_stuff::operator=(const medal_stuff &m) {
+ if (this != &m) {
+ if (promotion_text)
+ vm_free(promotion_text);
+ clone(m);
+ }
+ return *this;
+}
--- code/stats/medals.horig Sat May 14 14:05:49 2005
+++ code/stats/medals.h Sat May 14 14:57:36 2005
@@ -100,14 +100,18 @@
char bitmap[NAME_LENGTH];
int num_versions;
int kills_needed;
- int i;
//If this is a badge (kills_needed > 1)
char voice_base[MAX_FILENAME_LEN];
char *promotion_text;
- medal_stuff(){name[0]='\0';bitmap[0]='\0';num_versions=1;kills_needed=0;i=0;voice_base[0]='\0';promotion_text=NULL;}
- ~medal_stuff(){if(promotion_text != NULL){mprintf(("Freeing promo: %s\n",promotion_text));vm_free(promotion_text);}}
+ medal_stuff(){name[0]='\0';bitmap[0]='\0';num_versions=1;kills_needed=0;voice_base[0]='\0';promotion_text=NULL;}
+ medal_stuff(const medal_stuff &m) {clone(m);}
+ ~medal_stuff(){if(promotion_text)vm_free(promotion_text);}
+ const medal_stuff &operator=(const medal_stuff &m);
+
+private:
+ void clone(const medal_stuff &m);
} medal_stuff;
/*
typedef struct badge_stuff {
|
|
|
Here's a fix. Basically, you're using the destructor in a way that's incompatible with vector unless you also implement a copy constructor and an assignment operator. |
|
|
thats the problem, fixed |
|
|
It's crashing on startup now for me. Same basic problem though. Fizz's patch works fine for me but what got into CVS does not. |
|
|
The code that got into CVS is broken in several ways. The copy constructor implementation must at least set promotion_text to NULL before assignment. The assignment operator is broken as well (though only for the a = a case), and the "badfood" stuff really doesn't belong in production code... Also, IMHO, the scoping of temp_medal in my patch is much cleaner than the original clean/copy, clean/copy, clean/copy. I'm probably biased, though... And what's that "i" member for? It's not used anywhere, and poorly named anyway. |
|
|
i think the 0xbaadf00d thing is what MSVC sets everything to when a vector expands. The program will want to free it and crash - this 0x0325d1a8 + name 0x0325d1a8 "Epsilon Pegasi Liberation" + bitmap 0x0325d1c8 "Medal00.pcx" num_versions 4 kills_needed 0 + voice_base 0x0325d1f0 "" + promotion_text 0xbaadf00d "" |
|
|
That's very certainly because you don't NULL the promotion_text in the second constructor. |
|
|
makes sense. that damn java class ruined my C++ class knowledge. |
|
|
The "i" was some debugging stuff from me that slipped in with my vm_* commit. Doesn't serve any purpose. So, are you going to fix it phreak or should I just commit fizz's patch instead? I've been waiting since I thought you might do it but it sucks to leave it broken. I'll get the working patch in later tonight if you don't get there first. |
|
|
oh sorry forgot about it. commit what you have |
|
|
Ok, that should be it for this thing then. I can't get it to crash on start or exit. Fixered. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2005-05-12 09:14 | Goober5000 | New Issue | |
| 2005-05-12 13:16 | phreak | Note Added: 0002367 | |
| 2005-05-12 13:16 | phreak | Assigned To | phreak => WMCoolmon |
| 2005-05-12 15:52 | taylor | Note Added: 0002370 | |
| 2005-05-12 22:28 | taylor | Note Edited: 0002370 | |
| 2005-05-13 06:17 | WMCoolmon | Note Added: 0002375 | |
| 2005-05-13 06:22 | WMCoolmon | Note Edited: 0002375 | |
| 2005-05-13 19:44 | phreak | Note Added: 0002387 | |
| 2005-05-13 19:47 | phreak | Note Edited: 0002387 | |
| 2005-05-14 12:08 | fizz | File Added: fs2-medals.diff | |
| 2005-05-14 12:09 | fizz | Note Added: 0002396 | |
| 2005-05-14 15:09 | phreak | Note Added: 0002398 | |
| 2005-05-14 15:09 | phreak | Status | assigned => resolved |
| 2005-05-14 15:09 | phreak | Resolution | open => fixed |
| 2005-05-14 15:09 | phreak | Assigned To | WMCoolmon => phreak |
| 2005-05-15 06:06 | taylor | Status | resolved => feedback |
| 2005-05-15 06:06 | taylor | Resolution | fixed => reopened |
| 2005-05-15 06:06 | taylor | Note Added: 0002408 | |
| 2005-05-15 10:11 | fizz | Note Added: 0002412 | |
| 2005-05-15 15:02 | phreak | Note Added: 0002414 | |
| 2005-05-15 15:16 | fizz | Note Added: 0002415 | |
| 2005-05-16 02:15 | phreak | Note Added: 0002418 | |
| 2005-05-18 01:02 | taylor | Note Added: 0002422 | |
| 2005-05-18 03:36 | taylor | Status | feedback => assigned |
| 2005-05-18 03:36 | taylor | Assigned To | phreak => taylor |
| 2005-05-18 03:48 | phreak | Note Added: 0002426 | |
| 2005-05-18 12:58 | taylor | Status | assigned => resolved |
| 2005-05-18 12:58 | taylor | Resolution | reopened => fixed |
| 2005-05-18 12:58 | taylor | Note Added: 0002429 |