Memory corruption with fs::Sprite

[Jaenis]

I am using nvwa to track memory leaks and Sprite class (or Resource) seems delete a pointer twice when it goes out of scope.

I have following program

Code: [Select]


#include <SFML/Graphics.hpp>

int main()
{
// Create the main window
sf::RenderWindow App(sf::VideoMode(800, 600), "SFML window");

// Load image
sf::Image Image;
if (!Image.LoadFromFile("Image.png"))
return EXIT_FAILURE;

// Own context to isolate the error
{
// Create sprite
sf::Sprite Sprite(Image);
}  // Double deletion here

return EXIT_SUCCESS;
}


On the marked line nvwa will kill the application because deletion of unknown memory address.

I compiled this in MinGw 4.3.3 with SFML_DYNAMIC flag and I am using dynamic libraries (will get thousands of errors with static libs)
SFML version is 1.5, using debug versions (no change if I use non-debug)

Here's backtrace from gdb

Code: [Select]


...std stuff removed...
#9  0x004be99c in std::set<sf::ResourcePtr<sf::Image>*, std::less<sf::ResourcePtr<sf::Image>*>, std::allocator<sf::ResourcePtr<sf::Image>*> >::erase (this=0x22feac, __x=@0x22fae4)
    at c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../../include/c++/4.3.3-dw2-tdm-1/bits/stl_set.h:448
#10 0x00470a3e in sf::Resource<sf::Image>::Disconnect (this=0x22feac,Observer=@0x22fe94)
    at c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../../include/SFML/System/Resource.inl:87
#11 0x0046cc09 in ~ResourcePtr (this=0x22fe94)
    at c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../../include/SFML/System/ResourcePtr.inl:68
#12 0x0046cd95 in _fu0___ZTVN2sf6SpriteE ()
    at c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../../include/SFML/Graphics/Sprite.hpp:45
#13 0x004017d0 in main () at Src/main.cpp:17


So in short ~ResourcePtr() calls myResource->Disconnect(*this);
which then calls myObservers.erase(&Observer); (where observer is ResourcePtr) and this ends up to deletion of unknown memory addess.
Quite weird...

[Laurent]

So in short ~ResourcePtr() calls myResource->Disconnect(*this);
which then calls myObservers.erase(&Observer); (where observer is ResourcePtr) and this ends up to deletion of unknown memory addess.

Why "unknown memory address"? This is the address of the ResourcePtr which is being destroyed, I don't see any error there.
Plus, there's no dynamic allocation/deletion involved in this process (except the ones happening in std::set but there's no error there), I don't understand why your tool is reporting something.

will get thousands of errors with static libs

I'm curious to see them

[Jaenis]

So in short ~ResourcePtr() calls myResource->Disconnect(*this);
which then calls myObservers.erase(&Observer); (where observer is ResourcePtr) and this ends up to deletion of unknown memory addess.

Why "unknown memory address"? This is the address of the ResourcePtr which is being destroyed, I don't see any error there.
Plus, there's no dynamic allocation/deletion involved in this process (except the ones happening in std::set but there's no error there), I don't understand why your tool is reporting something.

I actually debugged this for a while and got into conclusion that this memory was allocated inside of dll, so nvwa could not see it. And therefore it reported unknown memory address deletion when this memory was deleted.
That std::set::erase command was in inl file, so it was compiled for my program and nvwa could track that. (nvwa overwrites new and delete operators, so it couldn't see memory allocation in external library)

So, I assume that this can be ignored now

will get thousands of errors with static libs

I'm curious to see them

Here you go, this is with g++ ver 4.3.3

Code: [Select]

C:\Code\Code\SFMLTest>make
g++ -c -o Obj/main.o Src/main.cpp -W -Wall -ISrc -MMD -Werror -ISrc/Lib
g++ -Wl,--enable-auto-import -o test.exe Obj/main.o -lmingw32 -lsfml-graphics-s-d -lsfml-window-s-d -lsfml-system-s-d
c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../libsfml-graphics-s-d.a(Sprite.o): In function `ZN2sf6SpriteC2Ev':
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:38: undefined reference to `__gxx_personality_sj0'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:38: undefined reference to `_Unwind_SjLj_Register'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:42: undefined reference to `_Unwind_SjLj_Resume'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:42: undefined reference to `_Unwind_SjLj_Unregister'
c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../libsfml-graphics-s-d.a(Sprite.o): In function `ZN2sf6SpriteC1Ev':
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:38: undefined reference to `__gxx_personality_sj0'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:38: undefined reference to `_Unwind_SjLj_Register'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:42: undefined reference to `_Unwind_SjLj_Resume'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/Sprite.cpp:42: undefined reference to `_Unwind_SjLj_Unregister'
...snip...
c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../libsfml-graphics-s-d.a(Sprite.o): In function `ZNSt8_Rb_treeIPN2sf11ResourcePtrINS0_5ImageEEES4_St9_IdentityIS4_ESt4lessIS4_ESaIS4_EE8_S_valueEPKSt18_Rb_tree_node_base':
D:/Programmes/CodeBlocks/bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_tree.h:(.text$_ZNSt8_Rb_treeIPN2sf11ResourcePtrINS0_5ImageEEES4_St9_IdentityIS4_ESt4lessIS4_ESaIS4_EE14_M_create_nodeERKS4_[std::_Rb_tree<sf::ResourcePtr<sf::Image>*, sf::ResourcePtr<sf::Image>*, std::_Identity<sf::ResourcePtr<sf::Image>*>, std::less<sf::ResourcePtr<sf::Image>*>, std::allocator<sf::ResourcePtr<sf::Image>*> >::_M_create_node(sf::ResourcePtr<sf::Image>* const&)]+0xc): undefined reference to `__gxx_personality_sj0'
..snip..
c:/code/mingw/bin/../lib/gcc/i686-pc-mingw32/4.3.3-dw2-tdm-1/../../../libsfml-graphics-s-d.a(RenderWindow.o): In function `ZN2sf12RenderWindowC2Ev':
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/RenderWindow.cpp:41: undefined reference to `__gxx_personality_sj0'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/RenderWindow.cpp:41: undefined reference to `_Unwind_SjLj_Register'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/RenderWindow.cpp:41: undefined reference to `_Unwind_SjLj_Resume'
D:/Programmation/Cpp/SFML/tags/1.5/src/SFML/Graphics/RenderWindow.cpp:41: undefined reference to `_Unwind_SjLj_Unregister'
..snip...

Seems that all errors are for undefined references to '__gxx_personality_sj0', '_Unwind_SjLj_Register', '_Unwind_SjLj_Resume' and '_Unwind_SjLj_Unregister'


Since that looks quite a lot like version error with compilers (SFML was compiled with ver 3.4.4 and mine is 4.3.3) I tried to compile with version 3.4.5, here's that:

Code: [Select]


C:\Code\Code\SFMLTest>g++ --version
g++ (GCC) 3.4.5 (mingw-vista special r3)
Copyright (C) 2004 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

C:\Code\Code\SFMLTest>make
g++ -c -o Obj/main.o Src/main.cpp -W -Wall -ISrc -MMD -Werror -ISrc/Lib
C:/Code/MinGW/bin/../lib/gcc/mingw32/3.4.5/../../../../include/SFML/System/ResourcePtr.inl: In destructor `sf::ResourcePtr< <template-parameter-1-1> >::~ResourcePtr() [with T = sf::Image]':
Src/main.cpp:159:   instantiated from here
C:/Code/MinGW/bin/../lib/gcc/mingw32/3.4.5/../../../../include/SFML/System/ResourcePtr.inl:68: error: no matching function for call to `sf::Image::Disconnect(sf::ResourcePtr<sf::Image>* const) const'
C:/Code/MinGW/bin/../lib/gcc/mingw32/3.4.5/../../../../include/SFML/System/Resource.inl:86: note: candidates are: void sf::Resource<T>::Disconnect(sf::ResourcePtr<T>&) const [with T = sf::Image]
make: *** [Obj/main.o] Error 1

Somehow it won't even compile

Maybe if I grab a version from SVN and compile that, then I probably could link it. Worth a try...

[Jaenis]

I got version from SVN and compiled it and now I can link with static libraries. And now nvwa could catch the memory allocation, so that error is gone.

Though it found a memory leak  :wink:

Code: [Select]


C:\Code\Code\SFMLTest>test.exe
Leaked object at 003328C0 (size 4, 00447C0B)
Leaked object at 00332930 (size 212, 004489BA)
*** 2 leaks found

C:\Code\Code\SFMLTest>addr2line -e test.exe 00447C0B 004489BA
C:\Code\Code\Libs\SFML\trunk\src\SFML\Window/Context.cpp:82
C:\Code\Code\Libs\SFML\trunk\src\SFML\Window/WindowImpl.cpp:61



Context.cpp:82:

Code: [Select]

static Context* GlobalContext = new Context; // Never deleted, on purpose

WindowImpl.cpp:59-62:

Code: [Select]

WindowImpl* WindowImpl::New()
{
    return new WindowImplType();
}
That WindowImpl::New gets called from that Context constructor, so that known "never deleted" line seems to be causing both of these leaks.

Would there be any way to actually delete that global context before program exit? Just to make this clean  

[Laurent]

It's in the roadmap, and it's already done in the sfml2 branch.

[Jaenis]

It's in the roadmap, and it's already done in the sfml2 branch.

Yes it is fixed there, just checked

And while doing that I found one weird thing... (in SFML2)
On the moment when I create sf::Music object, gdb tells me that loads of threads are been generated. Seems that every time I poll all events from the queue new thread is generated.

I have this kind of code

Code: [Select]


#include <SFML/Graphics.hpp>
#include <SFML/Audio.hpp>

int main()
   {
   // Create the main window
   sf::RenderWindow App(sf::VideoMode(800, 600), "SFML window");
   
   // Create music object
   printf("Create music\n");fflush(stdout);
   sf::Music Music;
   
   // Process "mainloop" ten times
   for (int i=0; i<10; i++)
      {
      // Sleep a bit to get events
      sf::Sleep(1);
     
      // Process all events
      sf::Event Event;
      while (App.GetEvent(Event))
         ;
     
      // One new thread has now been generated
      printf("Debug\n");fflush(stdout);
      }
   
   return EXIT_SUCCESS;
   }


Here's gdb output from this source

Code: [Select]

(gdb) run
Starting program: C:\Code\Code\SFMLTest/test.exe
[New thread 1608.0xde8]
[New thread 1608.0xe8c]
[New thread 1608.0xd70]
[New thread 1608.0x520]
[New thread 1608.0xb7c]
[New thread 1608.0xdd0]
[New thread 1608.0xaf8]
[New thread 1608.0x4f4]
Create music
[New thread 1608.0x620]
Debug
[New thread 1608.0x808]
Debug
[New thread 1608.0x644]
Debug
[New thread 1608.0xaf4]
Debug
[New thread 1608.0xe2c]
Debug
Debug
[New thread 1608.0xe3c]
[New thread 1608.0x42c]
Debug
[New thread 1608.0xadc]
Debug
Debug
[New thread 1608.0x140]
[New thread 1608.0xcb4]
Debug
[New thread 1608.0x624]

Program exited normally.
(gdb)

There is something fishy going on
Haven't checked sfml code yet what it does in this case, but it seems that creating Music class stars music output (even if  I haven't called Music::Play yet) and music output is generating lots of threads by some reason.

[Laurent]

That's weird, I can't see anything that could create all these threads endlessly.

[Jaenis]

This is quite weird...
When tracing it seems that every new thread just exists immediately. After running a while, gdb had only 7 threads running that were generated on startup.

And one even more weird thing.
It does not matter where that sf::Music line is. Even if it is defined in it's own context after the "main loop" those threads will be generated.

This starts to sound like a tool issue. Can you reproduce this error?
I am using gdb 6.8 from here: http://www.equation.com/servlet/equation.cmd?fa=gdb

[Jaenis]

Managed to narrow this thing a bit, here's current version:

Code: [Select]

#include <SFML/Window.hpp>
#include <../extlibs/headers/AL/alc.h>

// Create global alc device
ALCdevice *myDevice=alcOpenDevice(NULL);  // <---- This is the critical line

int main()
{
// <--- Creating alc device here causes same behavior

// Create the main window
sf::Window App(sf::VideoMode(800, 600), "SFML window");

// <--- Creating alc device here causes no problems

// Process "mainloop" ten times
for (int i=0; i<5; i++)
{
// Sleep a bit to get events
sf::Sleep(1);

// Process all events
sf::Event Event;
while (App.GetEvent(Event))
;

// One new thread has now been generated
printf("Debug\n");fflush(stdout);
}

return EXIT_SUCCESS;
}


This error happens when global instance of OpenAL is created. Normally done by SFML/Audio/AudioDevice.cpp.

If I move that alcOpenDevice call inside main function, before sf::Window call, nothing changes. But if it is after Window call, this behavior disappears. (see the highlighted lines on code example)

Could it be so that sf::Window call creates another thread (compared to global context) and that is causing context switching which in turn happens to generate extra threads by some reason.

So, now this seems to be either OpenAL threading or sf::Window bug.

[Jaenis]

Debugged this further.
If I comment out ProcessJoystickEvents() from WindowImpl::DoEvents() this bug disappears.

I am using Windows Vista (32bit), so this call goes to SFML\Window\Win32\Joystic.cpp : Joystick::UpdateState()
That function is calling windows joyGetDevCaps(...) which returns value 165

According to specs [ http://msdn.microsoft.com/en-us/library/ms709350(VS.85).aspx ]* it should return one of these values:
JOYERR_NOERROR = 0
MMSYSERR_NODRIVER = 6
MMSYSERR_INVALPARAM = 11

But it did return value 165, there are few discussions about that but nothing clear.
I am wondering if that value is some Vista internal value for undefined joystic, since I haven't ever connected one.

If I comment out that joyGetDevCaps(...) function (and do not enter the if statement) this bug disappears.


Also, since I don't have any joystics connected, why SFML is polling them on every loop?


* Forum software does not accept parenthesis in urls


Edit:
Just found that value 165 is JOYERR_PARMS.
Quite logical, you are polling joystic 1 and 2 when there is 0 joystics present.

[Jaenis]

Since windows and linux source differ on this case, I made a patch to make windows code similar to linux:

Code: [Select]


Index: branches/sfml2/src/SFML/Window/Win32/Joystick.cpp
===================================================================
--- branches/sfml2/src/SFML/Window/Win32/Joystick.cpp (revision 1194)
+++ branches/sfml2/src/SFML/Window/Win32/Joystick.cpp (working copy)
@@ -42,6 +42,7 @@
 void Joystick::Initialize(unsigned int index)
 {
     // Reset state
+    myPresent   = false;
     myIndex     = JOYSTICKID1;
     myNbAxes    = 0;
     myNbButtons = 0;
@@ -62,6 +63,7 @@
                 // Ok : store its parameters and return
                 JOYCAPS caps;
                 joyGetDevCaps(myIndex, &caps, sizeof(caps));
+                myPresent   = true;
                 myNbAxes    = caps.wNumAxes;
                 myNbButtons = caps.wNumButtons;
                 if (myNbButtons > JoystickState::MaxButtons)
@@ -84,6 +86,10 @@
 {
     JoystickState state = {0};
 
+ // Check if joystick is present
+    if (!myPresent)
+        return state;
+
     // Get the joystick caps (for range conversions)
     JOYCAPS caps;
     if (joyGetDevCaps(myIndex, &caps, sizeof(caps)) == JOYERR_NOERROR)
Index: branches/sfml2/src/SFML/Window/Win32/Joystick.hpp
===================================================================
--- branches/sfml2/src/SFML/Window/Win32/Joystick.hpp (revision 1194)
+++ branches/sfml2/src/SFML/Window/Win32/Joystick.hpp (working copy)
@@ -78,6 +78,7 @@
     ////////////////////////////////////////////////////////////
     // Member data
     ////////////////////////////////////////////////////////////
+    bool myPresent;           ///< Is joystick present
     unsigned int myIndex;     ///< Windows ID of the joystick
     unsigned int myNbAxes;    ///< Number of axis supported by the joystick
     unsigned int myNbButtons; ///< Number of buttons supported by the joystick

Edit: typo

[Laurent]

I applied your fix.

This is so weird... but anyway thanks a lot for debugging and fixing it

[Jaenis]

Great, thanks

Btw, I noticed that you applied this into trunk, though this error was present on SFML2 branch :wink:
I should have pointed that out more clearly, but this is nice to have in 1.5+ too.

[Laurent]

I know, but I couldn't find any reason not to apply this patch to 1.5 as well. I'll merge it into the sfml2 branch soon.