Results 1 to 4 of 4

Thread: Pointer parameter in signals: Memory leak?

  1. #1
    Join Date
    May 2006
    Location
    Stuttgart, Germany
    Posts
    22
    Thanks
    2
    Qt products
    Qt4
    Platforms
    Unix/X11 Windows

    Default Pointer parameter in signals: Memory leak?

    Hello,

    Qt Code:
    1. class Sender : public QObject
    2. {
    3. Q_OBJECT
    4.  
    5. public:
    6. Sender(int val) : mVal(val) { }
    7.  
    8. signal:
    9. void intChanged(ChangeObject*);
    10.  
    11. public slots:
    12. void changeInt(int i) {
    13. mVal = i;
    14. emit intChanged(new ChangeObject(mVal));
    15. }
    16.  
    17. private:
    18. int mVal;
    19. };
    20.  
    21. class Receiver : public QObject
    22. {
    23. Q_OBJECT
    24.  
    25. public:
    26. Receiver();
    27.  
    28. public slots:
    29. void onIntChanged(ChangeObject* c) {
    30. mReceivedVal = c->changed();
    31. }
    32.  
    33. private:
    34. mReceivedVal;
    35. };
    36. class ChangeObject
    37. {
    38. public:
    39. ChangeObject(int changed) : mChanged(changed) { }
    40.  
    41. int changed() const { return mChanged; }
    42. private:
    43. int mChanged;
    44. };
    To copy to clipboard, switch view to plain text mode 

    Where to delete new created ChangeObject? in class Receiver? What about not handled signals with pointer parameter types? (like the Qt own signals that are not accepted ie e->accept() is not called in receiver object)

    Thanks for any clarification

  2. #2
    Join Date
    Mar 2008
    Location
    Marslev, Denmark
    Posts
    31
    Thanked 2 Times in 2 Posts
    Qt products
    Qt4 Qt5 Qt/Embedded
    Platforms
    Unix/X11 Windows Android

    Default Re: Pointer parameter in signals: Memory leak?

    It's impossible to answer your question, because we can't figure out if the caller or callee is responsible or not.

    Unless you use queued connections, a signal-slot connection is just a function call. So it should be safe to delete the pointer at the end of the slot. If this is the case, I'd use auto_ptr. (Not that I would ever emit pointers unless I was absolutely forced to.) But if you are using queued connections, which is the case if it's across threads, then you have no idea if it's safe or not.

    Instead, you should recode your application to emit something that is not a pointer. In the code you offer, the pointer is only used to get a bool. In that case, emit the bool directly.

    If you are worried about performance, in case your object isn't just a bool but might be huge, you can create your own implicitly shared object type and have copy-on-write. If you have coded this const-correctly, that should fix your problems.

    You can also emit a QSharedPointer<whatever*>, but you might have to register this type to QVariant.

    Of all these solutions, there is only one that really makes sense: Don't emit pointers.
    Bo Thorsen, Viking Software
    Qt applications on Linux and Windows

  3. #3
    Join Date
    May 2006
    Location
    Stuttgart, Germany
    Posts
    22
    Thanks
    2
    Qt products
    Qt4
    Platforms
    Unix/X11 Windows

    Default Re: Pointer parameter in signals: Memory leak?

    Hello Bo,

    thanks for your answer.

    Of course in reality the signal parameter is meant to be an event object. I have a class hierarchy of special event objects:

    Qt Code:
    1. class Event
    2. {
    3. public:
    4. enum EventType
    5. {
    6. ET_Message,
    7. ET_Item,
    8. ET_Action,
    9. [...]
    10. };
    11.  
    12. Event(EventType t);
    13.  
    14. private:
    15. EventType mEventType;
    16. };
    17.  
    18. class MessageEvent : public Event
    19. {
    20. public:
    21. MessageEvent(Event::EventType t = Event::Message);
    22.  
    23. QString message() const {
    24. return mMessage;
    25. }
    26.  
    27. private:
    28. QString mMessage;
    29. };
    30.  
    31. class ItemEvent : public Event
    32. {
    33. public:
    34. ItemEvent(Item i, Event::EventType t = Event::Item);
    35.  
    36. Item item() const {
    37. return mItem;
    38. }
    39.  
    40. private:
    41. Item mItem;
    42. };
    To copy to clipboard, switch view to plain text mode 

    In my general state class I have a general signal to emit all types of events, any type of event can be assigned to any state:

    Qt Code:
    1. class State : public QObject
    2. {
    3. Q_OBJECT
    4. public:
    5. State(QObject* parent = 0);
    6.  
    7. void addEvent(KeyEvent k, Event* e);
    8.  
    9. signals:
    10. void event(Event*);
    11.  
    12. public slots:
    13. void handleEvent(KeyEvent k);
    14.  
    15. private:
    16. QHash<KeyEvent, Event*> mEventTable;
    17. };
    18.  
    19.  
    20. void State::handleEvent(KeyEvent k)
    21. {
    22. // emit event for this key event
    23. if (mEventTable.contains(k))
    24. emit event(mEventTable.value(k));
    25. }
    To copy to clipboard, switch view to plain text mode 

    Events are registered with states at creation:

    Qt Code:
    1. class MyOtherClass
    2. {
    3.  
    4. void initStates()
    5. {
    6. State* s1 = new State();
    7. s1->addEvent(KE_CursorDown, new ItemEvent(item1, Event::ET_Item));
    8. [...]
    9.  
    10. register all state signals:
    11. connect(s1, SIGNAL(event(Event*)), this, SLOT(onEvent(Event*)));
    12. [...]
    13. }
    14. };
    To copy to clipboard, switch view to plain text mode 

    So there are "polymorphic" events, so I have to use pointers I think. I want a general event signal for all sorts of events.

    Thanks for any suggestions

  4. #4
    Join Date
    Jan 2006
    Location
    Warsaw, Poland
    Posts
    33,359
    Thanks
    3
    Thanked 5,015 Times in 4,792 Posts
    Qt products
    Qt3 Qt4 Qt5 Qt/Embedded
    Platforms
    Unix/X11 Windows Android Maemo/MeeGo
    Wiki edits
    10

    Default Re: Pointer parameter in signals: Memory leak?

    I suggest to avoid calling your method "event" as there already exists a method with this name in QObject. As for the problem itself, I would probably try to avoid broadcasting your events with signals or at least avoiding pointers. There is a good chance at some point you forget what ownership rules you have set and delete the event in a wrong place. If you want to store the event objects somewhere then obviously you can't delete them in the context of the emitting method. Since you don't know how many objects may store the event you can't delete it in the receiver's context as well. So the only choice is to delete the object in the context of the object that created the event (or a global event manager) but you have to make sure no other context references your events. You can do it like so:

    Qt Code:
    1. void EventManager::cleanupAllEvents() {
    2. emit eventsAboutToBeDeleted();
    3. qDeleteAll(m_events);
    4. emit eventsDeleted();
    5. }
    To copy to clipboard, switch view to plain text mode 

    Then you can connect to the first signal with every object handling events and clear their own event caches.

    You can also do it on an event-by-event basis:
    Qt Code:
    1. void EventManager::cleanupEvent(Event *e){
    2. emit eventToBeDeleted(e);
    3. delete m_events.find(e, 0);
    4. m_events.removeOne(e);
    5. emit eventDeleted(e);
    6. }
    To copy to clipboard, switch view to plain text mode 

    and handle it in receivers in a similar fashion as in the previous case.

    But I'd suggest simply to avoid storing the events anywhere. It'd be best if you just used Qt's event system for your events, just subclass QEvent and use QCoreApplication::postEvent() to post your events.
    Your biological and technological distinctiveness will be added to our own. Resistance is futile.

    Please ask Qt related questions on the forum and not using private messages or visitor messages.


Similar Threads

  1. Replies: 14
    Last Post: 1st December 2009, 21:45
  2. Qt dll + memory leak
    By Fastman in forum Qt Programming
    Replies: 3
    Last Post: 2nd August 2009, 14:28
  3. QList<pointer>::takeFirst() and memory leak question
    By AcerExtensa in forum Qt Programming
    Replies: 3
    Last Post: 9th July 2009, 14:20
  4. Memory leak
    By vvbkumar in forum General Programming
    Replies: 4
    Last Post: 2nd September 2006, 16:31
  5. Memory leak
    By zlatko in forum Qt Programming
    Replies: 8
    Last Post: 28th March 2006, 20:02

Tags for this Thread

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  
Digia, Qt and their respective logos are trademarks of Digia Plc in Finland and/or other countries worldwide.