Memory problems?

[Laethnes]

I use this http://wyw.dcweb.cn/leakage.htm library to watch my memory while developing. It was ok until I begin use SFML. When I create simple project http://www.sfml-dev.org/documentation/1.6/ , memory watcher reports deleting object which was not created. With debugger I found it was this "if (!Music.OpenFromFile("nice_music.ogg"))" line. So I rebuild SFML myself to discover where it is, but when I linked this files, it was ok.
When I use any of them (debug, release, myself build), at the end, it reports memory leak. (One ptr is 4 and other 388 bytes big.)
So I would like to ask you, how you watch memory while creating this lib and if this problem is in SFML or if I do something wrong.
Btw. I wanted use this program to watch memory in SFML itself, but when I include debug_new.h in SFML Config.h file, compilation ends with error because of STL libraries. It's normal, but I do not feel like editing every file do undef and redefine new around including STL...

EDIT:
Ah, I'm sorry, I just found topics about this. In these topics was said that memory watch tool can be ended (by destructor, call from atexit...) before last memory is freed. I don't think it is this case, because in description of tool I use is written that if something is freed after report, it will print new info with new memory leakage.

EDIT:
Ah, I'm sorry, again :3. I forget mention closer informations:
SFML: 1.6
Compiler: MinGW-TDM 4.4.1, MS Visual Studio Express 2008
OS: Windows XP SP3

[Laethnes]

I thought ignoring my post means that everything is ok. And maybe it is, but I finally found the problem and I think you (sfml makers) should note us, memory watch users, because you HAVE never deleted memory.

Version: 1.6
File: SFML/Window/Context.cpp
Line: 77 - 85

Code: [Select]

////////////////////////////////////////////////////////////
/// Get the global context
////////////////////////////////////////////////////////////
Context& Context::GetGlobal()
{
    static Context* GlobalContext = new Context; // Never deleted, on purpose

    return *GlobalContext;
}


I thought that you never mentioned that SFML HAVE nevet deleter ptr means, I did something wrong and "GetGlobal()" shouldn't be called, but then I find code which makes it before main (saving into global static variable in not-named namespace - so hidden).

So I patched it for myself with Loki library

Code: [Select]

static Loki::SmartPtr<Context> GlobalContext = new Context;

(but I thing auto_ptr should work too) and now everything is ok and no leakage is reported. (Loki by default checks existence of pointer with assert when dereferencing, so if some method used this global context, it would notice me).

[Laurent]

Sorry for not replying sooner, actually I think that I missed your first post.

The GlobalContext is not deleted explicitely on purpose, so your patch will break this and will make the code crash in certain conditions, at global exit.

Note that the memory is freed by the OS anyway, so this is not really a memory leak (I mean, it is controlled and the "lost" memory never grows, it is a fixed amount of exactly sizeof(Context)). Only memory tools will notice it.

[Laethnes]

Sorry for not replying sooner, actually I think that I missed your first post.

The GlobalContext is not deleted explicitely on purpose, so your patch will break this and will make the code crash in certain conditions, at global exit.

Note that the memory is freed by the OS anyway, so this is not really a memory leak (I mean, it is controlled and the "lost" memory never grows, it is a fixed amount of exactly sizeof(Context)). Only memory tools will notice it.

That is why in my previous post didn't write that it is "memory leak", because, like you wrote and like it is in code, it is on purpose. My point was my memory watch tool still write me that there is a memory leak, but on the other hand, when I searched internet, I found answer like "we do not have leaks, because we use memory watch tool and we deallocate some pointers after main() ends, so some tools can detect false leaks", but this - even on purpose - is always detected, so it should be noted - IMHO.
Well, like I wrote, Loki::SmartPtr does checks with assert, so "return *GlobalContext;" calls - in SmartPtr class - assert(pointer) and if it is destroyed (zero), it abort code (and note in std out) - and it will not crash. And I should mention I did this fix only in debug version.

[Laurent]

Well, like I wrote, Loki::SmartPtr does checks with assert, so "return *GlobalContext;" calls - in SmartPtr class - assert(pointer) and if it is destroyed (zero), it abort code (and note in std out) - and it will not crash. And I should mention I did this fix only in debug version.

Yes but it will also delete the pointer when reaching the end of its lifetime, which must not happen.

[Laethnes]

Well, like I wrote, Loki::SmartPtr does checks with assert, so "return *GlobalContext;" calls - in SmartPtr class - assert(pointer) and if it is destroyed (zero), it abort code (and note in std out) - and it will not crash. And I should mention I did this fix only in debug version.

Yes but it will also delete the pointer when reaching the end of its lifetime, which must not happen.

Yeah, but thanks to nature of Loki I can make simple workaround. It is template based - and SmartPtr is also template.

Code: [Select]

SmartPtr<SavedType, HowAndWhenItIsDestroyed, HowCanBePtrConverted, HowIsPtrCheckedWhileReferencing, Storing, SomethingAboutConstness> ptr;

So I can write simple storage class, use it and set pointer to NULL in destructor. (This is main thought of Loki - with simple class I can alter how main Loki class does what it does.)

[Laurent]

Yeah, I remember this design from the "modern C++ design" book that I read several years ago

But if you set the pointer to NULL instead of deleting it, what's the purpose of using a smart pointer? Does it make your memory tool happy (it shouldn't, the memory still leaks)?

[Laethnes]

Yeah, I remember this design from the "modern C++ design" book that I read several years ago

But if you set the pointer to NULL instead of deleting it, what's the purpose of using a smart pointer? Does it make your memory tool happy (it shouldn't, the memory still leaks)?

No, I change storage policy class to set pointer to NULL in its destructor, but it will not change destructor of SmartPtr and it will (as always) call other (policy) to find if it should be destroyed, and if yes (which is this case), it will call storage policy class to destroy. And I can still let it destroy, but also, in its destructor set pointer to NULL, so when it will be dereferenced after destroying SmartPtr, assert check could prevent using wrong memory. (Of course this suppose that memory of destroyed static object is not used until real end of program ... but with this I can do nothing.)

So this is not about setting pointer to NULL instead of destroying it, but setting to NULL after it's destroyed.

So
- pointer is destroyed at the end of program
- if some other static object in its destructor want to use this pointer, assert in SmartPtr should prevent using it, because it is set to NULL

Btw. that books author is genius, isn't he :3

[Laethnes]

Hm, here is practical example:

Code: [Select]


    // This is default storage policy Smart Ptr uses (because I left here original comments, I mark my with "Laethnes:")

    template <class T>
    class DefaultSPStorage
    {
    public:
        typedef T* StoredType;    // the type of the pointee_ object
        typedef T* InitPointerType; /// type used to declare OwnershipPolicy type.
        typedef T* PointerType;   // type returned by operator->
        typedef T& ReferenceType; // type returned by operator*

        DefaultSPStorage() : pointee_(Default())
        {}

        // Laethnes: this is what I'm talking about: this is only
        // change I need to do to make sure the pointer is
        // NULL after static pointer is destroyed
        ~DefaultSPStorage()
        {
            pointee_ = NULL;
        }

        // The storage policy doesn't initialize the stored pointer
        //     which will be initialized by the OwnershipPolicy's Clone fn
        DefaultSPStorage(const DefaultSPStorage&) : pointee_(0)
        {}

        template <class U>
        DefaultSPStorage(const DefaultSPStorage<U>&) : pointee_(0)
        {}

        DefaultSPStorage(const StoredType& p) : pointee_(p) {}

        PointerType operator->() const { return pointee_; }

        ReferenceType operator*() const { return *pointee_; }

        void Swap(DefaultSPStorage& rhs)
        { std::swap(pointee_, rhs.pointee_); }

        // Accessors
        template <class F>
        friend typename DefaultSPStorage<F>::PointerType GetImpl(const DefaultSPStorage<F>& sp);

        template <class F>
        friend const typename DefaultSPStorage<F>::StoredType& GetImplRef(const DefaultSPStorage<F>& sp);

        template <class F>
        friend typename DefaultSPStorage<F>::StoredType& GetImplRef(DefaultSPStorage<F>& sp);

    protected:
        // Destroys the data stored
        // (Destruction might be taken over by the OwnershipPolicy)
//
// If your compiler gives you a warning in this area while
// compiling the tests, it is on purpose, please ignore it.
        void Destroy()
        {
            // Laethnes: This destroys the pointer and I will not change it
            delete pointee_;
        }

        // Default value to initialize the pointer
        static StoredType Default()
        { return 0; }

    private:
        // Laethnes: HERE is pointer stored
        // Data
        StoredType pointee_;
    };


[Laurent]

- pointer is destroyed at the end of program

But what I'm saying from the beginning is that this object mustn't be destroyed

Preventing other objects from using it after it is destroyed won't help, actually the object is never used. Its main purpose is to create a global OpenGL context which lives as long as OpenGL calls are made -- and some of them can happen in the destructor of global objects.

So the problem is not about accessing this particular object safely, it is really to keep it alive even at global exit.

Btw. that books author is genius, isn't he :3

Yep, very famous guy in the C++ community.

[Laethnes]

- pointer is destroyed at the end of program

But what I'm saying from the beginning is that this object mustn't be destroyed

Preventing other objects from using it after it is destroyed won't help, actually the object is never used. Its main purpose is to create a global OpenGL context which lives as long as OpenGL calls are made -- and some of them can happen in the destructor of global objects.

I know and I wrote as answer that after pointer is destroyed, it is also set to NULL and assert check in SmartPtr aborts program if something like that happens (and so it will not crash nor send segfault). And because (I also wrote that) I use this "fix" in debug version only, I do not care about it until something like that really happens - which so far did not. (And if calling OpenGL functions after end of main() is fully in my hands, I would consider it as error anyway.)

[Laurent]

Sorry (really), but I still don't get it. What does your "fix" fix exactly? Which issue do try to solve with your code?

I know and I wrote as answer that after pointer is destroyed, it is also set to NULL and assert check in SmartPtr aborts program if something like that happens (and so it will not crash nor send segfault)

But we don't care about this object, really. Nobody tries to use it.
It will crash because OpenGL calls happening at global exit will find no valid context to use.

[Laethnes]

Sorry (really), but I still don't get it. What does your "fix" fix exactly? Which issue do try to solve with your code?

I know and I wrote as answer that after pointer is destroyed, it is also set to NULL and assert check in SmartPtr aborts program if something like that happens (and so it will not crash nor send segfault)

But we don't care about this object, really. Nobody tries to use it.
It will crash because OpenGL calls happening at global exit will find no valid context to use.

I see!!! So that is the issue! I didn't get it until now, I'm sorry.

Hm... so that needs second thought :3.

[Laurent]

Don't try to find a workaround, the current code works and there's nothing that you can do to make it "better". This issue is already fixed properly in SFML 2.

[Laethnes]

Don't try to find a workaround, the current code works and there's nothing that you can do to make it "better". This issue is already fixed properly in SFML 2.

Ok, I just didn't know what it was about.