Why am I getting errors freeing memory allocated by apriltags image_u8_create() and stored in 2D vec-CodePudding

I am attempting to detect apriltags in image streams. Because the images come in from multiple sources at a high rate, detecting the tags in the image callback will take too long, causing images to be dropped due to missed callbacks. I have decided to store images for a few seconds at a time, and run detection on the images afterwards. Between each run of images, I would like to free all the used memory, as I will need to store multiple GB of data for each ~5 second run and images/framerates/sources change between runs.

I am using the image_u8_t type that comes with the apriltag library compiled from source:

typedef struct image_u8 image_u8_t;
struct image_u8
{
    const int32_t width;
    const int32_t height;
    const int32_t stride;
    uint8_t *buf;
};

and which has create() and destroy() functions (create() is a wrapper that fills in some default values for the shown create_from_stride(), namely stride = width:

image_u8_t *image_u8_create_stride(unsigned int width, unsigned int height, unsigned int stride)
{
    uint8_t *buf = calloc(height*stride, sizeof(uint8_t));

    // const initializer
    image_u8_t tmp = { .width = width, .height = height, .stride = stride, .buf = buf };

    image_u8_t *im = calloc(1, sizeof(image_u8_t));
    memcpy(im, &tmp, sizeof(image_u8_t));
    return im;
}

void image_u8_destroy(image_u8_t *im)
{
    if (!im)
        return;
    free(im->buf);
    free(im);
}

The first run of images always goes as expected, however, on the second, I consistently get errors freeing memory. It seems that although the vectors report a size=0 after using clear(), it is retaining values at the front of the vector and iterating through them, attempting to double free memory. A minimum example of code that also shows the error is below:

#include <iostream>
#include <apriltag/apriltag.h>
#include <vector>

using namespace std;

vector<vector<image_u8_t *>> images(2);

void create_images(int i){
  image_u8_t * img;
  img = image_u8_create(1920, 1200);
  images.at(i).push_back(img);
}

int main() {
  char c;

  for (int i = 0; i < 3; i  ){
    for (int j = 0; j < 2; j  ){
      create_images(j);
    }
  }
  // This works fine
  for (auto vec : images){
    for (auto img : vec){
      image_u8_destroy(img);
    }
    vec.clear();
  }
  // Just to pause and inspect output
  cin >> c;

  for (int i = 0; i < 3; i  ){
    for (int j = 0; j < 2; j  ){
      create_images(j);
    }
  }
  // This causes a segfault/free() error 
  for (auto vec : images){
    for (auto img : vec){
      image_u8_destroy(img);
    }
    vec.clear();
  }

}

Printing the pointer to be freed (im->buf) shows what seems to be happening:

Freeing image buffer at **0x7ff8cf22e010**
Freeing image buffer at 0x7ff8cedc8010
Freeing image buffer at 0x7ff8ce962010
Freeing image buffer at 0x7ff8ceffb010
Freeing image buffer at 0x7ff8ceb95010
Freeing image buffer at 0x7ff8ce72f010
c
Freeing image buffer at **0x7ff8cf22e010**
Segmentation fault (core dumped)

and the output from my real program shows a more specific but similar problem:

img u8 vector length: 92
Destroyed all images and cleared vector. New size = 0
Destroyed all images and cleared vector. New size = 0
Destroyed all images and cleared vector. New size = 0
Destroyed all images and cleared vector. New size = 0
Freeing image buffer at 0x7f2834000b80
free(): invalid pointer

Can anyone explain if I am misunderstanding how vectors work, the clear() function specifically, or point me towards where I might be causing this issue?

Editing to add output that shows even after clearing and having size() return 0, on the next push_back()s, the old values seem to reappear in the vector:

Vector size before 1st clear: 3
Freeing image buffer at 0x7f5e34fe6010
Freeing image buffer at 0x7f5e34b80010
Freeing image buffer at 0x7f5e3471a010
Vector size after 1st clear: 0
Vector size before 1st clear: 3
Freeing image buffer at 0x7f5e34db3010
Freeing image buffer at 0x7f5e3494d010
Freeing image buffer at 0x7f5e344e7010
Vector size after 1st clear: 0
c
Vector size before 2nd clear: 6
Freeing image buffer at 0x7f5e34fe6010
Segmentation fault (core dumped)

CodePudding user response：

// This works fine
  for (auto vec : images){
    for (auto img : vec){
      image_u8_destroy(img);
    }
    vec.clear();
  }

LOOKS like it works fine but auto vec in for (auto vec : images) is a value, not a reference. It makes a copy of the vector in images, and that means vec.clear(); cleared a copy. The original in images still contains image_u8 instances holding now-dangling pointers.

If I'd been paying attention to the

Vector size before 2nd clear: 6

diagnostic, I'd have figured this out an hour ago when the question was first asked. Good debugging. Asker was looking at the right stuff and just missed a detail that surprises a lot of people. In C unless you ask for a reference, or you're passing around arrays, you get a value.

Solution:

// This REALLY works fine
  for (auto &vec : images){
    for (auto &img : vec){ // optional. You probably won't save much since `img` is 
                           // already a pointer
      image_u8_destroy(img);
    }
    vec.clear();
  }