Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 11 of 11

Thread: Command Processor Object lifetime

  1. #1
    Junior Member
    Join Date
    Jan 2020
    Posts
    12

    Command Processor Object lifetime

    I am using gcode to command a machine. A Teensy 4.0 is the controller recieving the data via serial and places them in a queue. When nothing is processing i pop a command from the queue.

    I have three variables: previousc, currentc and nextc. At the beginning of each loop I move currentc to previousc nextc to currentc and then I pop a command from the queue it gets stored in the nextc variable.

    My controller crashes when acessing the nextc variable (when doing a NULL check in case the buffer is empty). I have boiled it down to the code below but i dont understand why it crashes. My guess is that it is about object lifetime but i dont know... What is wrong?

    Code:
    class moveXY
    { 
      public:
      int x; 
      int y;
    };
    
    moveXY * nextMove; 
    moveXY _nextMove = *new moveXY();
       
    void setup() {
      _nextMove.x = 88;
      nextMove = &_nextMove;
    }
    
    void loop() {
        _nextMove = * new moveXY();
        _nextMove.x = 883;
        nextMove = &_nextMove;
        Serial.print("\n _nextMove = ");Serial.print(_nextMove.x);
        if(nextMove)
          Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
        delay(1000);
      
        nextMove = NULL;
    
        if(nextMove)  
          Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
    
    }

  2. #2
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,081
    This :
    moveXY * nextMove;

    Shows that nextMove is a pointer. Setting that pointer to NULL with :
    nextMove = NULL;

    Means any write to that null ptr will fault.

    Not seeing where that is in above code as it seems to only read when it has a valid pointer - but setting nextMove = NULL; could do it.

    Also those move vars are static/global - there seems to be a lot of "new" going on.

    Not shown any output? How long does it run before crashing? Adding CrashReport?

  3. #3
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,743
    Code:
        if(nextMove)
          Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
    Just an observation. You haven't parenthesized the scope of the if statement and therefore only the first .print is inside the if. This means you will print "\n nextMove = " if nextMove is not null. But you will always print (*nextMove).x.
    It should, presumably, be:
    Code:
        if(nextMove) {
          Serial.print("\n nextMove = ");
          Serial.print((*nextMove).x);
        }
    Pete
    Last edited by el_supremo; 08-15-2021 at 05:20 PM. Reason: punctuation!

  4. #4
    Junior Member
    Join Date
    Jan 2020
    Posts
    12
    Thanks.... The oneliner if was the fault I have been banging my head for 2 days at this boildown.... feeling stupid now... But this means my error is else where.... Will continiue digging... TNKS!!

  5. #5
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,081
    Quote Originally Posted by DanielO View Post
    Thanks.... The oneliner if was the fault I have been banging my head for 2 days at this boildown.... feeling stupid now... But this means my error is else where.... Will continiue digging... TNKS!!
    Good catch Pete! ... missed the double statement line in reading

  6. #6
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,743
    And another observation
    Code:
        nextMove = NULL;
    
        if(nextMove)  
          Serial.print("\n nextMove = ");Serial.print((*nextMove).x);
    Even if this if statement is parenthesized, this will never print anything because you set nextMove to NULL before the if statement.

    Pete

  7. #7
    Senior Member
    Join Date
    May 2015
    Location
    USA
    Posts
    1,071
    As an exercise, I recommend you do two things:

    a) explain in words exactly what "moveXY _nextMove = *new moveXY();" does

    b) explain when one should use the "delete" operator

  8. #8
    Junior Member
    Join Date
    Jan 2020
    Posts
    12
    Finally got on the right track!
    Because I used new bufStruct() when adding to the buffer (in a separate class reading of serial and taking the buffer as reference) I never realised them being temporary. And reading about it brought my attention to be able to use the copyfunction of the default generated constructor of structs.
    I was using:
    Code:
        old = active;
        active = upcoming;
        upcoming= &(commandBuffer.shift()));
    The next iteration of the loop will have destroyed the object pointed to by upcoming making (*active).data return garbage.


    Code:
        
    
    delete old; 
        old = active;
        active = upcoming;
        upcoming= new bufStruct((commandBuffer.shift()));
    By using new above I no longer have a temporary and am therefore responsible for cleanup - hence "delete old"

    Hope this helps someone!
    Last edited by DanielO; 08-15-2021 at 11:29 PM. Reason: Big fingers :)

  9. #9
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    15,081
    Working on something else @KurtE discovered that the NULL Ptr write trap/fault extends to bytes 0-31 and covers both READ and WRITE to those addresses.

    That explains the fault when this nextMove pointer was NULL :: Serial.print((*nextMove).x);

  10. #10
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,290
    Quote Originally Posted by defragster View Post
    Working on something else @KurtE discovered that the NULL Ptr write trap/fault extends to bytes 0-31 and covers both READ and WRITE to those addresses.

    That explains the fault when this nextMove pointer was NULL :: Serial.print((*nextMove).x);
    Yes, the mpu region is 32 bytes minimum.
    There is ITCM at 0 anyway, so it is OK.
    One could go ahead and cover the whole ITCM (for writes).. i don't think anybody uses self-modyfing code today. And if its really needed, the trap can be disabled.

  11. #11
    Senior Member
    Join Date
    May 2015
    Location
    USA
    Posts
    1,071
    > upcoming= new bufStruct((commandBuffer.shift()));

    As with malloc(), one should always check that "new" succeeded.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •