Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Help - Random heap corruptions.  (Read 7327 times)

0 Members and 1 Guest are viewing this topic.

Vot1_Bear

  • Newbie
  • *
  • Posts: 31
  • Noob in-training
    • View Profile
    • Game development blog
Help - Random heap corruptions.
« on: September 10, 2013, 01:04:28 pm »
Hello SFML Community!
This question is a bit long, so I'll try to summarize it first as briefly as possible.

I'm having problem with my game project, Blastorium.

It runs finely on my machine, but will randomly stop working (went not responding) in other machines. I'm using Windows 7, but the errors show no consistency on OS or specs. The most likely cause is heap corruption, as I did suffer from it some time ago (but it was solved on my machine).

My project's github can be found here, but as a disclaimer, I am an extreme beginner in C++ with nothing much of a guide on coding a program, so the code you're going to see is downright horribly untidy and unstructured.

Since I suspect scanning through all the files won't be feasible for most of you, here's the pseudocode:
Quote
Files used:
-Main.cpp (main)
-Interface.cpp (runs functions such as transition screens and main menu screen)
-Select.h/cpp (class, runs the select screen, that is, the part where you select weapons/stage)
-Tilelist and Texturemanager.h/cpp (class, manages sprites/textures and rendering individual sprites)
-Globals.h/cpp (class, contains a shared_ptr<> of all the classes below as for global access)
-Engine.h/cpp (class, the main game loop runs here. Calls most of other classes below)
-Player, Level, Powerups, WeaponManager, WpnBomb/Mine/Rocket/Medi .cpp/h (classes, maintains the aspects of the game such as players/powerups/etc, and also runs their Logic() and Render() functions every loop)
Each class below Engine.cpp has their own tilemanager and texturemanager, meaning they load files themselves and handles their own logic and rendering. Engine.cpp only calls them

Every class with the exception of main menu GUI classes (SelectManager) and utility classes (TileList, TileManager) is handled by GlobalManager class.
The GlobalManager class contains a shared_ptr<> to each of those classes, and each is initialized immediately once the app was run. Later on in the game. Engine.h will fill each of those classes with the necessary information based on user inputs (Which map, which weapons, etc). This is re-done every game loop, e.g. from start to game over.

Pseudocode of what's happening when you run it
  • Run Main.cpp
  • Main.cpp initializes Engine class, SelectManager class (select screen) and GlobalManager class
  • Main.cpp calls MainMenu function (not a class) from Interface.cpp
  • MainMenu() draws all the gears and receive input, returning an int
  • If it signals a play command, main menu runs the SelectScreen function from SelectManager
  • SelectScreen function receives input and returns accordingly
  • Main.cpp runs the Go function of the Engine class, initiating the game
  • Engine.cpp initializes every other class for a game round (levels, player, weapons, etc)
  • Each respective classes loads their own files
  • In each game loop, Engine calls the Logic and Render function of each class
  • If a player's HP reaches zero, Engine calls the FinishMatch function of Data class
  • Data class prints numbers then returns
  • Engine then calls the EndGameChoice function (not from a class), which receives user input
  • -If it selects play again, engine refreshes every class and runs again
    -If it selects main menu, engine stops and the program returns to Main.cpp, to no.3 to be precise
    -If it selects exit then the game exits accordingly

Heap corruptions commonly happens when you close the game (The last step, when you press exit) but in rare cases, during the initialization of the game after select screen (step no.7-9)

Do note that in previous versions, there were no select or main screens (meaning the program skips steps no. 3 to 6) and the game runs just fine. Perhaps that holds some relevance.

Anything that might help would be appreciated. As of now, I still hold very little knowledge on good/bad programming habits, or tracing the source of these errors, and programming in general.

I know that my coding is horrible and that the problem most likely stems from an incorrect structure of the program itself, but I really need help with it.. Most tutorials on the internet only covers the basics of a single file and not how they are supposed to work together, and I'm out of ideas on how to fix the problem (especially since it runs fine on my machine, making debugging difficult).

Another question, though not as pressing, is performance. Is there a reason why my game still somewhat lags at 30FPS even on machines that runs 3d games just fine? I'm pretty sure my code is not that horrible (or maybe it is) to cause such drop in performance

Thanks in advance, and sorry if the question is too much or lacking information.
« Last Edit: September 10, 2013, 01:11:15 pm by Vot1_Bear »

Ixrec

  • Hero Member
  • *****
  • Posts: 1241
    • View Profile
    • Email
Re: Help - Random heap corruptions.
« Reply #1 on: September 10, 2013, 05:35:42 pm »
I'm probably nowhere near smart enough to figure out what's really going wrong here, but one thing that jumps out to me immediately is that a lot of these shared_ptr's probably should be unique_ptr's.  I'll tell you if I see anything else later.

slotdev

  • Sr. Member
  • ****
  • Posts: 385
    • View Profile
Re: Help - Random heap corruptions.
« Reply #2 on: September 10, 2013, 05:54:33 pm »
This could be a red herring, but I've had a similar problem on other machines, which I found to be caused by not having OpenAL installed. Grab it from https://dl.dropboxusercontent.com/u/17968394/oalinst.zip and see if that helps.

Failing that, if you run a Debug build on the W7 machine, does it also go wrong?

Maybe run some sanity checks - disable large chunks of the code until it runs 100% OK, and then start to add things back in, one by one until it breaks.
SFML 2.1

Ixrec

  • Hero Member
  • *****
  • Posts: 1241
    • View Profile
    • Email
Re: Help - Random heap corruptions.
« Reply #3 on: September 10, 2013, 06:19:03 pm »
Maybe run some sanity checks - disable large chunks of the code until it runs 100% OK, and then start to add things back in, one by one until it breaks.

He probably can't do that because the errors are unpredictable, which is probably why he's asking us for help in the first place.

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Help - Random heap corruptions.
« Reply #4 on: September 10, 2013, 06:28:27 pm »
but as a disclaimer, I am an extreme beginner in C++ with nothing much of a guide on coding a program, so the code you're going to see is downright horribly untidy and unstructured.
This is where you should start fixing your program.

Make sure you get rid of some very error-prone low-level techniques: Manual memory management, arrays, global variables, C functions like printf(), rule of three (it's rarely necessary).

Your code is full of other questionable things, for example: #define for constants, return; at the end of void functions, declarations far from the initialization and usage, non-portable backslashes instead of slashes in paths, using namespace (especially in headers), exit() calls, assignments instead of constructor initializer lists, non-standard headers such as <string.h>, magic numbers everywhere, ...

When you care more about code quality, runtime errors like the one you mentioned will become very rare, and you can focus on actual logic bugs ;)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Ixrec

  • Hero Member
  • *****
  • Posts: 1241
    • View Profile
    • Email
Re: Help - Random heap corruptions.
« Reply #5 on: September 10, 2013, 07:54:04 pm »
Kind of off-topic, but this could be important for my code someday, so...

error-prone low-level techniques [such as] rule of three (it's rarely necessary).

Simply put: What's so error-prone and unnecessary about it?  I haven't felt the need to add rule of three to my own code yet but I thought I was just being lazy like every other newbie.  I've never seen anyone claim it was anything other than a best practice.

Vot1_Bear

  • Newbie
  • *
  • Posts: 31
  • Noob in-training
    • View Profile
    • Game development blog
Re: Help - Random heap corruptions.
« Reply #6 on: September 11, 2013, 02:39:38 am »

Make sure you get rid of some very error-prone low-level techniques: Manual memory management,
Obliterated every single * pointers (i think) since they caused a bug earlier. Except the one in XML reads, those somehow lead to errors when I change it.


arrays, global variables
Should I replace every array with vector, and can I know the reason why array is unsafe or something? Thanks.
Global variables was everywhere in the non-recoded version (and so were problems). It should be clean now though


C functions like printf()
The only ones I use is at the start of terminal. Will obliterate once help screen is finished, thanks.

rule of three (it's rarely necessary).

Can you elaborate what this is? I'd like to avoid that too.

E:Nevermind

 
Your code is full of other questionable things, for example: #define for constants, return; at the end of void functions, declarations far from the initialization and usage, non-portable backslashes instead of slashes in paths, using namespace (especially in headers), exit() calls, assignments instead of constructor initializer lists, non-standard headers such as <string.h>, magic numbers everywhere, ...

When you care more about code quality, runtime errors like the one you mentioned will become very rare, and you can focus on actual logic bugs ;)
Most of them (exit, return in end of void) resulted from me running out of ideas to fix the bug... But the rest does come from my poor coding habits. Will fix the rest soon, thanks!

I think slashes have been fixed thanks to texus, by the way.

Thanks for the tips!
« Last Edit: September 11, 2013, 04:24:02 am by Vot1_Bear »

Gobbles

  • Full Member
  • ***
  • Posts: 132
    • View Profile
    • Email
Re: Help - Random heap corruptions.
« Reply #7 on: September 11, 2013, 02:44:35 am »
Have you made sure your not leaking memory?

If not you can use this:
//DEBUG CODE
#ifdef _DEBUG
#define _CRTDBG_MAP_ALLOC
#include <crtdbg.h>
#define DEBUG_NEW new(_CLIENT_BLOCK, __FILE__, __LINE__)
#define new DEBUG_NEW
#endif

int main()
{
        _CrtSetDbgFlag ( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF );

This will allow you to see any leaked memory when you close the program
« Last Edit: September 11, 2013, 02:46:38 am by Gobbles »

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Help - Random heap corruptions.
« Reply #8 on: September 11, 2013, 11:50:26 am »
Obliterated every single * pointers (i think) since they caused a bug earlier.
Don't avoid pointers in general, they are fine when they do not own memory. Avoid new and delete.
int a;
int* p = &a; // good
int* q = new int; // bad


Should I replace every array with vector, and can I know the reason why array is unsafe or something? Thanks.
std::vector when the size is dynamic, std::array otherwise.

Arrays are bad, because:
  • they don't provide index/iterator checks in debug mode
  • they don't have value semantics (you cannot copy, assign, pass to or return from functions)
  • they are not very typesafe, since they can be implicitly converted to a pointer
  • they don't have size(), begin() or end() member functions
std::array fixes all these problems without any disadvantages. In particular, std::array is not slower than C arrays (in release mode).


Nevermind
You should not avoid the rule of three, but follow it ;) What I meant is that it's rarely necessary to write a user-defined destructor, copy constructor and assignment operator, especially if you apply the RAII idiom. Letting the compiler generate these functions not only saves time, but it also prevents bugs like a forgotten member.


Thanks for the tips!
You're welcome, glad I could help :)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Silvah

  • Guest
Re: Help - Random heap corruptions.
« Reply #9 on: September 11, 2013, 03:10:06 pm »
non-standard headers such as <string.h>
Minor nitpick: <string.h> is standard, as per D.5 and table 154.

You should not avoid the rule of three, but follow it ;) What I meant is that it's rarely necessary to write a user-defined destructor, copy constructor and assignment operator, especially if you apply the RAII idiom. Letting the compiler generate these functions not only saves time, but it also prevents bugs like a forgotten member.
So you mean the rule of zero?

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Help - Random heap corruptions.
« Reply #10 on: September 11, 2013, 05:44:13 pm »
Minor nitpick: <string.h> is standard, as per D.5 and table 154.
Ah, it's inherited from the C standard. But I'd recommend to use <cstring> in C++ (if the C functions are needed at all); then the functions are also accessible via std namespace.

So you mean the rule of zero?
Very good article, thanks!

However, std::unique_ptr and std::shared_ptr are of limited utility when it comes to avoiding the three/five, simply because they don't maintain copy semantics of the surrounding object. In order to cope with these limitations, I have written aurora::CopiedPtr, a smart pointer I frequently use in Thor. In short, it is able to deep-copy polymorphically:
aurora::CopiedPtr<Base> origin(new Derived);
aurora::CopiedPtr<Base> copy = origin; // invokes Derived copy constructor
« Last Edit: September 11, 2013, 05:47:51 pm by Nexus »
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Vot1_Bear

  • Newbie
  • *
  • Posts: 31
  • Noob in-training
    • View Profile
    • Game development blog
Re: Help - Random heap corruptions.
« Reply #11 on: September 12, 2013, 03:09:14 am »
Updated the repository, that should fix some of them (mainly return; exit(); and define constants)
I still suck at using const though. I can't figure out how to make them usable across multiple header files... They can generate errors if two consts over different files have the same name, but they don't work otherwise.

Oh, and
Arrays are bad, because:
  • they don't provide index/iterator checks in debug mode
  • they don't have value semantics (you cannot copy, assign, pass to or return from functions)
  • they are not very typesafe, since they can be implicitly converted to a pointer
  • they don't have size(), begin() or end() member functions
std::array fixes all these problems without any disadvantages. In particular, std::array is not slower than C arrays (in release mode).
Still working on it, thanks for the information!

Still wondering about the heap corruption thing though. It works fine with me with either builds, and
Have you made sure your not leaking memory?

If not you can use this:
//DEBUG CODE
#ifdef _DEBUG
#define _CRTDBG_MAP_ALLOC
#include <crtdbg.h>
#define DEBUG_NEW new(_CLIENT_BLOCK, __FILE__, __LINE__)
#define new DEBUG_NEW
#endif

int main()
{
        _CrtSetDbgFlag ( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF );

This will allow you to see any leaked memory when you close the program
doesn't show anything, apparently.

I need to get another machine to test it out.

Gobbles

  • Full Member
  • ***
  • Posts: 132
    • View Profile
    • Email
Re: Help - Random heap corruptions.
« Reply #12 on: September 12, 2013, 04:02:43 am »
hmm I can't test too much right now, but tomorrow at work I should have some down time, I'll use the opportunity to take a closer look.

Vot1_Bear

  • Newbie
  • *
  • Posts: 31
  • Noob in-training
    • View Profile
    • Game development blog
Re: Help - Random heap corruptions.
« Reply #13 on: September 13, 2013, 04:10:04 pm »
Sorry for unnecessarily bumping this, but I believe I still need help regarding the heap corruptions.

Again, it still runs fine on my computer, and I'm having difficulty with finding machines that produces the error on exit/play. Even if I did fix it I'd like to know the cause, since fixing through random trial-and-error changes isn't really a good way to improve myself in the long run.

If it means anything, here are my major suspicions for bug causes:
-The main menu function being a separate function instead of a class (leading to weird memory problems with the texture). I'm planning to make it into another class anyway, but does this assumption has any chance of being true?
-Select.h (and future main menu.h, maybe) not contained in global.h.
-The RenderWindow being held by every class via shared_ptr(s) even though I could use it through global.h?
-Library/external requirements? Kind of unreasonable, but I don't think anyone in SFML has encountered the problem... leading me to believe the errors are due to a lack of required file(s) in some machines
-Plainly due to my horrible code which is probably buggy somewhere

Again, I could probably try them one-by-one myself, but I would really appreciate expert insight on the situation, for the previous replies has greatly helped me in improving myself. Thanks!