Results 1 to 8 of 8

Thread: QList::erase method does not work if I include an "emit signal" instruction

  1. #1

    Default QList::erase method does not work if I include an "emit signal" instruction

    I have the following QList:

    Qt Code:
    1. QList<PendingMsg> list[MAX_CLIENTS];
    To copy to clipboard, switch view to plain text mode 

    And I want to erase some elements from it when a specific time has passed (a typical timeout). Every time an element is removed a signal is emitted, however I don't know why, but the code in charge of erasing the elements does not work when it uses the signal I have defined to do this. The iterator does not iterate the list properly (for example if there is only one element, the for loop runs for 3 iterations, instead of 1, and then a memory access violation is thrown), however, f I only comment the "emit signal" line, everything works...

    This is the code in charge of deleting the elements of the list:

    Qt Code:
    1. void PendingList::checkTimeouts()
    2. {
    3. qint64 t1 = QDateTime::currentMSecsSinceEpoch();
    4. for (int i = 0; i < MAX_CLIENTS; i++)
    5. {
    6. QList<PendingMsg>::iterator it;
    7. for (it = list[i].begin(); it != list[i].end(); )
    8. {
    9. quint64 t0 = it->request.getTimeStamp();
    10. quint64 dift = t1 - t0;
    11. if (dift >= it->timeout)
    12. {
    13. CMD cmd1 = it->cmd;
    14. CMD cmd2 = it->request.getCmd();
    15. it = list[i].erase(it);
    16. emitTimeout((System)i, cmd1, cmd2);
    17. }
    18. else
    19. it++;
    20. }
    21. }
    22. stopIfEmpty();
    23. }
    To copy to clipboard, switch view to plain text mode 

    The emitTimeout function emits a signal depending on the parameters cmd1 and cmd2. If I comment this line, everything works..., moreover if I put there for example "emit timeout_ack_unit_ctrl(System)" where timeout_ack_unit_ctrl(System) is a signal declared, it does not work, on the other hand, if I try with the signal "timeout(System)" which is another signal declared, it does work... Can anyone tell me what's happening???

    You can see below the whole class:

    Header file:

    Qt Code:
    1. #pragma once
    2.  
    3. #include "MM.h"
    4. #include "Message.h"
    5. #include <QTimer>
    6. #include <QObject>
    7.  
    8. struct PendingMsg
    9. {
    10. Message request; //message pending to receive a response
    11. CMD cmd; // type of the pending response {ACK or ORD_RESP}
    12. int timeout; // Time that can be stored the message before emitting a timeout signal
    13.  
    14. PendingMsg(Message m, CMD c, int t)
    15. {
    16. request = m;
    17. cmd = c;
    18. timeout = t;
    19. };
    20.  
    21. QString display() const
    22. {
    23. return "[" + cmdTags[cmd] + " (" + cmdTags[request.getCmd()] + ")]";
    24. };
    25. };
    26.  
    27. class PendingList : public QObject
    28. {
    29. Q_OBJECT
    30.  
    31. public:
    32. PendingList(QObject* parent, int time);
    33. ~PendingList(void);
    34.  
    35. bool add(System sys, Message req, CMD cmd, int time);
    36. bool remove(System sys, Message msg);
    37.  
    38. void setCheckTimeInterval(int t) { checkTimeInterval = t;};
    39.  
    40. int size() const;
    41. void display() const;
    42.  
    43. bool resumeTimer(){return false;};
    44. bool pauseTimer(){return false;};
    45.  
    46. private:
    47.  
    48. int checkTimeInterval;
    49. QTimer timer;
    50.  
    51. QList<PendingMsg> list[MAX_CLIENTS];
    52.  
    53. // finds the position of the command cmd inside list[sys]
    54. int match(System sys, Message msg) const;
    55.  
    56. bool startIfEmpty();
    57. bool stopIfEmpty();
    58.  
    59. // Reports that the system 'sys' has not sent a message of type 'cmd1' as answer of a message of type 'cmd2'
    60. void emitTimeout(System sys, CMD cmd1, CMD cmd2);
    61.  
    62. void emitMatch(System sys, Message msg, Message req);
    63.  
    64. private slots:
    65.  
    66. void checkTimeouts();
    67.  
    68. signals:
    69.  
    70. void timeout(System);
    71. void timeout_ack_unit_ctrl(System);
    72. void timeout_ack_insp_mis(System);
    73. void timeout_ack_mosaic(System);
    74. void timeout_ack_weed_map(System);
    75. void timeout_ack_tre_mis(System);
    76. void timeout_ack_pause(System);
    77. void timeout_ack_stop(System);
    78. void timeout_ack_resume(System);
    79. void timeout_resp_insp_mis(System);
    80. void timeout_resp_mosaic(System);
    81. void timeout_resp_weed_map(System);
    82. void timeout_resp_tre_mis(System);
    83. void timeout_resp_pause(System);
    84. void timeout_resp_stop(System);
    85. void timeout_resp_resume(System);
    86.  
    87. void ack_insp_mis(System);
    88. void ack_mosaic(System);
    89. void ack_weed_map(System);
    90. void ack_tre_mis(System);
    91. void ack_unit_ctrl(System);
    92. void ack_stop(System);
    93. void ack_resume(System);
    94. void ack_pause(System);
    95.  
    96. void resp_insp_mis_ok(System);
    97. void resp_mosaic_ok(System);
    98. void resp_weed_map_ok(System);
    99. void resp_tre_mis_ok(System);
    100. void resp_stop_ok(System);
    101. void resp_resume_ok(System);
    102. void resp_pause_ok(System);
    103.  
    104. void resp_insp_mis_ko(System);
    105. void resp_mosaic_ko(System);
    106. void resp_weed_map_ko(System);
    107. void resp_tre_mis_ko(System);
    108. void resp_stop_ko(System);
    109. void resp_resume_ko(System);
    110. void resp_pause_ko(System);
    111. };
    To copy to clipboard, switch view to plain text mode 

    Cpp file:

    Qt Code:
    1. #include "PendingList.h"
    2.  
    3. PendingList::PendingList(QObject *parent, int time)
    4. {
    5. this->setParent(parent);
    6. checkTimeInterval = time;
    7. connect(&timer, SIGNAL(timeout()), this, SLOT(checkTimeouts()));
    8. }
    9.  
    10. PendingList::~PendingList(void)
    11. {
    12. }
    13.  
    14. bool PendingList::startIfEmpty()
    15. {
    16. if (this->size() > 0 && !timer.isActive()){
    17. timer.start(checkTimeInterval);
    18. return true;
    19. }
    20. return false;
    21. }
    22.  
    23. bool PendingList::stopIfEmpty()
    24. {
    25. if (this->size() == 0 && timer.isActive()){
    26. timer.stop();
    27. return true;
    28. }
    29. return false;
    30. }
    31.  
    32. void PendingList::display() const
    33. {
    34. printInfo(0, 0, false, false, "Pending List:");
    35. for (int i = 0; i < MAX_CLIENTS; i++)
    36. {
    37. int n = list[i].size();
    38. if (n > 0)
    39. {
    40. QString line = " - " + sysNames[i] + ": ";
    41. for (int j = 0; j < n; j++)
    42. {
    43. line += list[i][j].display();
    44. }
    45. printInfo(0, 0, false, false, line);
    46. }
    47. }
    48. }
    49.  
    50. int PendingList::match(System sys, Message msg) const
    51. {
    52. int psys = int(sys);
    53. int n = list[psys].size();
    54. for (int i = 0; i < n; i++)
    55. {
    56. CMD stCmd = list[psys][i].cmd;
    57. CMD cmd = msg.getCmd();
    58. if ((list[psys][i].cmd == msg.getCmd()) &&
    59. ((msg.getCmd() == ACK) || (msg.getCmd() == RESP && (msg.getPldByte(0) == list[psys][i].request.getCmd()))))
    60. return i;
    61. }
    62. return -1;
    63. }
    64.  
    65. bool PendingList::add(System sys, Message req, CMD cmd, int time)
    66. {
    67. quint8 ba[] = {req.getCmd()};
    68. Message msg(cmd, ba, 1);
    69. int p = match(sys, msg);
    70. if (p == -1)
    71. {
    72. int psys = int(sys);
    73. PendingMsg pm(req, cmd, time);
    74. list[psys].append(pm);
    75.  
    76. startIfEmpty();
    77. return true;
    78. }
    79. return false;
    80. }
    81.  
    82. bool PendingList::remove(System sys, Message msg)
    83. {
    84. int pos = this->match(sys, msg);
    85. if (pos < 0)
    86. return false;
    87. emitMatch(sys, msg, list[sys][pos].request);
    88. list[sys].removeAt(pos);
    89. stopIfEmpty();
    90. return true;
    91. }
    92.  
    93. int PendingList::size() const
    94. {
    95. int n = 0;
    96. for (int i = 0; i < MAX_CLIENTS; i++)
    97. n = n + list[i].size();
    98. return n;
    99. }
    100.  
    101. void PendingList::checkTimeouts()
    102. {
    103. qint64 t1 = QDateTime::currentMSecsSinceEpoch();
    104. for (int i = 0; i < MAX_CLIENTS; i++)
    105. {
    106. QList<PendingMsg>::iterator it;
    107. for (it = list[i].begin(); it != list[i].end(); )
    108. {
    109. quint64 t0 = it->request.getTimeStamp();
    110. quint64 dift = t1 - t0;
    111. if (dift >= it->timeout)
    112. {
    113. CMD cmd1 = it->cmd;
    114. CMD cmd2 = it->request.getCmd();
    115. it = list[i].erase(it);
    116. emitTimeout((System)i, cmd1, cmd2);
    117. }
    118. else
    119. it++;
    120. }
    121. }
    122. stopIfEmpty();
    123. }
    124.  
    125. void PendingList::emitTimeout(System sys, CMD cmd1, CMD cmd2)
    126. {
    127. printInfo(1, 1, true, true, "Timeout: Pending message (" + cmdTags[cmd1] + " from " + sysNames[sys] + ") erased from the pending messages list");
    128. if (cmd1 == ACK)
    129. switch (cmd2
    130. {
    131. case INSP_MIS:
    132. emit timeout_ack_insp_mis(sys);
    133. break;
    134. case MOSAIC:
    135. emit timeout_ack_mosaic(sys);
    136. break;
    137. case WEED_MAP:
    138. emit timeout_ack_weed_map(sys);
    139. break;
    140. case TRE_MIS:
    141. emit timeout_ack_tre_mis(sys);
    142. break;
    143. case PAUSE_MIS:
    144. emit timeout_ack_pause(sys);
    145. break;
    146. case STOP_MIS:
    147. emit timeout_ack_stop(sys);
    148. break;
    149. case RES_MIS:
    150. emit timeout_ack_resume(sys);
    151. break;
    152. }
    153. else // ORD_RESP
    154. switch (cmd2)
    155. {
    156. case INSP_MIS:
    157. emit timeout_resp_insp_mis(sys);
    158. break;
    159. case MOSAIC:
    160. emit timeout_resp_mosaic(sys);
    161. break;
    162. case WEED_MAP:
    163. emit timeout_resp_weed_map(sys);
    164. break;
    165. case TRE_MIS:
    166. emit timeout_resp_tre_mis(sys);
    167. break;
    168. case PAUSE_MIS:
    169. emit timeout_resp_pause(sys);
    170. break;
    171. case STOP_MIS:
    172. emit timeout_resp_stop(sys);
    173. break;
    174. case RES_MIS:
    175. emit timeout_resp_resume(sys);
    176. break;
    177. }
    178. }
    179.  
    180. void PendingList::emitMatch(System sys, Message msg, Message pendMsg)
    181. {
    182. CMD pendCmd = pendMsg.getCmd();
    183. if (msg.getCmd() == ACK)
    184. switch (pendMsg.getCmd())
    185. {
    186. case INSP_MIS:
    187. emit ack_insp_mis(sys);
    188. break;
    189. case MOSAIC:
    190. emit ack_mosaic(sys);
    191. break;
    192. case WEED_MAP:
    193. emit ack_weed_map(sys);
    194. break;
    195. case TRE_MIS:
    196. emit ack_tre_mis(sys);
    197. break;
    198. case PAUSE_MIS:
    199. emit ack_pause(sys);
    200. break;
    201. case STOP_MIS:
    202. emit ack_stop(sys);
    203. break;
    204. case RES_MIS:
    205. emit ack_resume(sys);
    206. break;
    207. default:
    208. if (pendMsg.is_UNIT_CTRL())
    209. emit ack_unit_ctrl(sys);
    210. break;
    211. }
    212. else // ORD_RESP
    213. if (msg.getPldByte(1) == OK)
    214. switch (pendCmd)
    215. {
    216. case INSP_MIS:
    217. emit resp_insp_mis_ok(sys);
    218. break;
    219. case MOSAIC:
    220. emit resp_mosaic_ok(sys);
    221. break;
    222. case WEED_MAP:
    223. emit resp_weed_map_ok(sys);
    224. break;
    225. case TRE_MIS:
    226. emit resp_tre_mis_ok(sys);
    227. break;
    228. case PAUSE_MIS:
    229. emit resp_pause_ok(sys);
    230. break;
    231. case STOP_MIS:
    232. emit resp_stop_ok(sys);
    233. break;
    234. case RES_MIS:
    235. emit resp_resume_ok(sys);
    236. break;
    237. }
    238. else
    239. switch (pendCmd)
    240. {
    241. case INSP_MIS:
    242. emit resp_insp_mis_ko(sys);
    243. break;
    244. case MOSAIC:
    245. emit resp_mosaic_ko(sys);
    246. break;
    247. case WEED_MAP:
    248. emit resp_weed_map_ko(sys);
    249. break;
    250. case TRE_MIS:
    251. emit resp_tre_mis_ko(sys);
    252. break;
    253. case PAUSE_MIS:
    254. emit resp_pause_ko(sys);
    255. break;
    256. case STOP_MIS:
    257. emit resp_stop_ko(sys);
    258. break;
    259. case RES_MIS:
    260. emit resp_resume_ko(sys);
    261. break;
    262. }
    263. }
    To copy to clipboard, switch view to plain text mode 

  2. #2
    Join Date
    Jan 2008
    Location
    Alameda, CA, USA
    Posts
    5,230
    Thanks
    302
    Thanked 864 Times in 851 Posts
    Qt products
    Qt5
    Platforms
    Windows

    Default Re: QList::erase method does not work if I include an "emit signal" instruction

    When you emit a signal, you return control to the Qt event loop. If you do not emit any signal from within checkTimeouts(), then your loops run completely to the end and checkTimeouts() exits before control returns to the event loop. When you emit the signal, then all pending events will be processed, including any new timeouts, which means you could be entering checkTimeouts() recursively. So basically, your list gets completely screwed up.

    You can probably verify this by putting some qDebug() statements at the beginning and end of checkTimeouts() and see if you get an "entering checkTimeouts()" and "exiting checkTimeouts()" occurring together in pairs of enter / exit. If you get two enters in a row, you have a second timeout being processed before the first one has finished.

    Time to re-think your design. Your basic problem is that it is taking longer to process all of the pending messages than the timeout period, so your timeouts are stacking up. So if your application allows you to ignore extra timeouts, then one easy way to avoid the problem is to keep a bool member variable in your PendingList class:

    Qt Code:
    1. void PendingList::checkTimeouts()
    2. {
    3.  
    4. qDebug() << "Entering checkTimeouts()";
    5.  
    6. if ( !inTimeout )
    7. {
    8. inTimeout = true; // this is a member variable of PendingList; initialize it to false in the constructor
    9.  
    10. qint64 t1 = QDateTime::currentMSecsSinceEpoch();
    11. for (int i = 0; i < MAX_CLIENTS; i++)
    12. {
    13. QList<PendingMsg>::iterator it;
    14. for (it = list[i].begin(); it != list[i].end(); )
    15. {
    16. quint64 t0 = it->request.getTimeStamp();
    17. quint64 dift = t1 - t0;
    18. if (dift >= it->timeout)
    19. {
    20. CMD cmd1 = it->cmd;
    21. CMD cmd2 = it->request.getCmd();
    22. it = list[i].erase(it);
    23. emitTimeout((System)i, cmd1, cmd2);
    24. }
    25. else
    26. it++;
    27. }
    28. }
    29. stopIfEmpty();
    30.  
    31. inTimeout = false;
    32. }
    33.  
    34. qDebug() << "Exiting checkTimeouts()";
    35. }
    To copy to clipboard, switch view to plain text mode 

    Another way to do it would be to stop the timer when you enter checkTimeouts() and start it when you exit. Or make it a single-shot, and simply restart it when you exit. That way you can be guaranteed that no new timeouts can be generated while you are processing the first one.
    Last edited by d_stranz; 1st May 2013 at 01:31.

  3. #3
    Join Date
    Sep 2011
    Posts
    1,241
    Thanks
    3
    Thanked 127 Times in 126 Posts
    Qt products
    Qt4
    Platforms
    Windows

    Default Re: QList::erase method does not work if I include an "emit signal" instruction

    no one talking about the elephant? Why are you instantiating a raw array of QList? Which voice in your head thought that was a good idea?
    If you have a problem, CUT and PASTE your code. Do not retype or simplify it. Give a COMPLETE and COMPILABLE example of your problem. Otherwise we are all guessing the problem from a fabrication where relevant details are often missing.

  4. #4
    Join Date
    Jan 2008
    Location
    Alameda, CA, USA
    Posts
    5,230
    Thanks
    302
    Thanked 864 Times in 851 Posts
    Qt products
    Qt5
    Platforms
    Windows

    Default Re: QList::erase method does not work if I include an "emit signal" instruction

    no one talking about the elephant?
    I thought it better to tackle things one step at a time. On the other hand, there's nothing basically wrong with declaring an array of QList<>. It will simply consist of an array of size MAX_CLIENTS of empty QList<> allocated on the stack. Nothing magic about that, and nothing that requires use of QVector< QList< PendingMsg > > or some other fancier data structure. Each QList<> in the array can be manipulated as if it were a standalone QList<>.

    I am not sure about the copy semantics, though. I am not sure if accessing "list[i]" results in the return of a copy or of a reference. If it is a copy, then that would be pretty inefficient.

  5. #5
    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: QList::erase method does not work if I include an "emit signal" instruction

    Quote Originally Posted by d_stranz View Post
    When you emit a signal, you return control to the Qt event loop. If you do not emit any signal from within checkTimeouts(), then your loops run completely to the end and checkTimeouts() exits before control returns to the event loop. When you emit the signal, then all pending events will be processed, including any new timeouts
    No, that's not true, at least when a single execution thread is involved. The only way that the method is re-entered is that the signal is connected to a slot which calls this method again (e.g. emitTimeout() is connected to checkTimeouts()).
    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.


  6. The following user says thank you to wysota for this useful post:

    d_stranz (2nd May 2013)

  7. #6
    Join Date
    Jan 2008
    Location
    Alameda, CA, USA
    Posts
    5,230
    Thanks
    302
    Thanked 864 Times in 851 Posts
    Qt products
    Qt5
    Platforms
    Windows

    Default Re: QList::erase method does not work if I include an "emit signal" instruction

    No, that's not true, at least when a single execution thread is involved. The only way that the method is re-entered is that the signal is connected to a slot which calls this method again (e.g. emitTimeout() is connected to checkTimeouts()).
    Yes, of course you're right, and my little qDebug() test would have proven me wrong. Blame it on coffee - either not enough or too much.

    So that seems to render my whole premise false; presuming that the OP isn't calling checkTimeouts() recursively, then why *does* emitting the signal cause the list to be improperly processed? Maybe one of the slots is also manipulating the list?

  8. #7
    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: QList::erase method does not work if I include an "emit signal" instruction

    Quote Originally Posted by d_stranz View Post
    then why *does* emitting the signal cause the list to be improperly processed?
    We don't know if the list is not processed correctly. We'd need to reproduce this errorneous behaviour which is not possible without a small compilable example. I'd start by disconnecting all the signals emitted from within emitTimeout() to see if the problem persists.
    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.


  9. #8

    Default Re: QList::erase method does not work if I include an "emit signal" instruction

    Hello all, sorry for the delay, I couldn't test my code until today.

    You can probably verify this by putting some qDebug() statements at the beginning and end of checkTimeouts() and see if you get an "entering checkTimeouts()" and "exiting checkTimeouts()" occurring together in pairs of enter / exit. If you get two enters in a row, you have a second timeout being processed before the first one has finished.
    I put the qDebug() statements and I got pairs enter / exit, so I wasn't receiving any secondary timeout.

    no one talking about the elephant? Why are you instantiating a raw array of QList? Which voice in your head thought that was a good idea?
    My system has always the same number (MAX_CLIENTS) of connections to the same number of systems, so I don't need a vector or anything else which can increase/decrease dinamically, I just need an array with MAX_CLIENTS cells.

    The only way that the method is re-entered is that the signal is connected to a slot which calls this method again (e.g. emitTimeout() is connected to checkTimeouts()).
    Indeed that was the problem. emitTimeout() wasn't connected directley to checkTimeouts() but it was connected to a slot which added a new item in the pendingList structure and then, the timer started again as well as checkTimeouts(). This kind of infinite loop ended up crashing my program and I think getting a bad behaviour when I debugged the program to see how it was processing the list (this point is not very clear to me because I don't know very well how Qt internally manages the signals). Once I deleted the insertion of the new element when the timeout arised everything went ok.

    Thank you very much for your help and sorry for the inconveniences caused.


    Added after 1 42 minutes:


    I have discovered another error which generated the bad behaviour when processing the list after the timeout was emitted. The timeout signal launched several slots and in a row and, inside of one of them, I called the QList::clear method, that's why, when the control returned to the checkTimeout method, the iterator wasn't pointing to a valid address.
    Last edited by ClintEastwood; 3rd May 2013 at 17:03.

Similar Threads

  1. Replies: 2
    Last Post: 27th January 2012, 17:29
  2. Why the signal "dataChanged" by QTableWidget not work?
    By Tao Clark in forum Qt Programming
    Replies: 1
    Last Post: 20th August 2011, 17:16
  3. Translation QFileDialog standart buttons ("Open"/"Save"/"Cancel")
    By victor.yacovlev in forum Qt Programming
    Replies: 4
    Last Post: 24th January 2008, 19:05
  4. Signal defined in "a.h" can not emit in "b.cpp"
    By Shawn in forum Qt Programming
    Replies: 9
    Last Post: 21st May 2007, 16:55
  5. Replies: 6
    Last Post: 3rd November 2006, 11:53

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.