invalid pointer when writing blank file

[Joe Falcioni]

When opening a file for write and closing it before writing any messages to it there is an invalid pointer cleanup. I occasionally encounter this issue when running a data logging script when there are no messages sent in the script run duration.

Code to replicate issue

Vector::BLF::File logfile;
logfile.open("test.blf", std::ios_base::out);
logfile.close();

Output

*** Error in `...pilot_models': free(): invalid pointer: 0x00007fffff2174d8 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fc56dad47e5]
/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fc56dadd37a]
/lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7fc56dae153c]
/usr/local/lib/libVector_BLF.so.2(_ZN6Vector3BLF16UncompressedFile11dropOldDataEv+0xe8)[0x7fc56e45eb38]
/usr/local/lib/libVector_BLF.so.2(_ZN6Vector3BLF4File31uncompressedFile2CompressedFileEv+0xdf)[0x7fc56e4504ef]
/usr/local/lib/libVector_BLF.so.2(_ZN6Vector3BLF4File25compressedFileWriteThreadEPS1_+0x28)[0x7fc56e450888]
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xd0840)[0x7fc56e10f840]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x76ba)[0x7fc56d62d6ba]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7fc56db6441d]

‌

[Tobias Lorenz]

I'll try to find the cause today.

[Joe Falcioni]

In the process of trying to determine if I needed to release the file after closing with a file.clear() equivalent, I stumbled across another issue. Here I’m not able to write more than 11 messages. I imagine the queue is filling up and then the program terminates.

I wasn’t sure if having the start time and message timestamps was required for writing multiple messages so I’ve included the steps I have taken to write those. I’m not sure why this hasn’t been a problem for me in other cases.

‌

#include <iostream>
#include <sys/time.h>
#include <stdio.h>
#include <chrono>
#include <Vector/BLF.h>

int main() {

  std::cout << "Start" << std::endl;

  auto * canMessage = new Vector::BLF::CanMessage;
  canMessage->channel = 1;
  canMessage->flags = 1; // TX
  canMessage->dlc = 2;
  canMessage->id = 0x33;
  canMessage->data[0] = 0x44;
  canMessage->data[1] = 0x55;

  std::cout << "Message created" << std::endl;

  time_t current_time_t;
  struct tm * tm;
  std::chrono::steady_clock::time_point log_start_sysclock;

  time(&current_time_t);

  //TODO: restructure to eliminate duplicate code
  Vector::BLF::File file;
  file.open("test.blf", std::ios_base::out);
  log_start_sysclock = std::chrono::steady_clock::now();
  tm = gmtime(&current_time_t);

  Vector::BLF::FileStatistics * blf_stats = &file.fileStatistics;
  blf_stats->measurementStartTime.year = tm->tm_year+1900;
  blf_stats->measurementStartTime.month = tm->tm_mon+1;
  blf_stats->measurementStartTime.day = tm->tm_mday;
  blf_stats->measurementStartTime.hour = tm->tm_hour;
  blf_stats->measurementStartTime.minute = tm->tm_min;
  blf_stats->measurementStartTime.second = tm->tm_sec;
  blf_stats->measurementStartTime.milliseconds = 0;

  if(file.is_open())
    std::cout << "Opened" << std::endl;

  for(int i=0; i<50; i++){
    std::chrono::steady_clock::time_point log_event_sysclock = std::chrono::steady_clock::now();

    canMessage->channel = 1;
    canMessage->flags = 1; // TX
    canMessage->dlc = 2;
    canMessage->id = 0x33;
    canMessage->data[0] = 0x44;
    canMessage->data[1] = 0x55;
    canMessage->objectTimeStamp = std::chrono::duration_cast<std::chrono::microseconds>(
              log_event_sysclock - log_start_sysclock).count() * 1000;
    file.write(canMessage);
    std::cout << "Write" << std::endl;
  }

  file.close();
  std::cout << "Closed" << std::endl;


  if (rename("test.blf", "firstlog.blf") != 0) {
    std::cout << "New log file failed" << std::endl;
  }else {

    std::cout << "New log file created" << std::endl;
  }
}

‌

Program Output

‌

Start
Message created
Opened
Write
Write
Write
Write
Write
Write
Write
Write
Write
Write
Write
Write

‌

[Tobias Lorenz]

Hi Joe,

the first issue I found. I fixed it and added a test case with this commit: https://bitbucket.org/tobylorenz/vector_blf/commits/5a7a1f8fb1aec5dd9a0f08afadedee25715ff992

Now, I’ll see for the second issue.

Bye

Tobias

[Tobias Lorenz]

Hi Joe,

The issue in your example is likely, that you give up ownership on the canMessage. You are not supposed to use it further one, but this is currently the case in the loop.

Can you check if it works, if you create a new message at the start of each loop?

To clarify this, I have on my todo list to change from plain C pointers to std::unique_ptr, because this has a clear ownership given. But this would require a complete rewrite, ending in a new major version.

Bye

Tobias

[Joe Falcioni]

Hi Tobias,

Thank you for your quick responses.

Is this what you suggest? The issue still exists when opening the next file.

‌

int main() {
  std::cout << "Start" << std::endl;

  //TODO: restructure to eliminate duplicate code
  Vector::BLF::File file;
  file.open("test.blf", std::ios_base::out);

  if(file.is_open())
    std::cout << "Opened" << std::endl;

  for(int i=0; i<50; i++){
    auto * canMessage = new Vector::BLF::CanMessage;
    canMessage->channel = 1;
    canMessage->flags = 1; // TX
    canMessage->dlc = 2;
    canMessage->id = 0x33;
    canMessage->data[0] = 0x44;

    file.write(canMessage);
    std::cout << "Write: " << i << std::endl;
  }

  std::cout << "Written" << std::endl;
  file.close();
  std::cout << "Closed" << std::endl;

  if (rename("test.blf", "firstlog.blf") != 0) {
    std::cout << "New log file failed" << std::endl;
  } else {
    std::cout << "New log file created" << std::endl;
  }

  file.open("test.blf", std::ios_base::out);

  for(int i=0; i<50; i++){
    auto * canMessage = new Vector::BLF::CanMessage;
    canMessage->channel = 1;
    canMessage->flags = 1; // TX
    canMessage->dlc = 2;
    canMessage->id = 0x33;
    canMessage->data[0] = 0x44;

    file.write(canMessage);
    std::cout << "Write: " << i << std::endl;
  }

  file.close();
  std::cout << "File closed:" << std::endl;

}

‌

‌

Start
Opened
Write: 0
Write: 1
Write: 2
Write: 3
Write: 4
Write: 5
Write: 6
Write: 7
Write: 8
Write: 9
Write: 10
Write: 11
Write: 12
Write: 13
Write: 14
Write: 15
Write: 16
Write: 17
Write: 18
Write: 19
Write: 20
Write: 21
Write: 22
Write: 23
Write: 24
Write: 25
Write: 26
Write: 27
Write: 28
Write: 29
Write: 30
Write: 31
Write: 32
Write: 33
Write: 34
Write: 35
Write: 36
Write: 37
Write: 38
Write: 39
Write: 40
Write: 41
Write: 42
Write: 43
Write: 44
Write: 45
Write: 46
Write: 47
Write: 48
Write: 49
Written
Closed
New log file created
Write: 0
Write: 1
Write: 2
Write: 3
Write: 4
Write: 5
Write: 6
Write: 7
Write: 8
Write: 9
Write: 10

‌

[Tobias Lorenz]

Hi Joe,

I didn’t had in mind to reuse the same File object to write another file. I can check where this needs additional initialization.

But you can easily fix this, when just creating a new File object, like this:

‌

int main()
{
    std::cout << "Start" << std::endl;
    {
        Vector::BLF::File file;
        file.open("test.blf", std::ios_base::out);

        if (file.is_open()) {
            std::cout << "Opened" << std::endl;
        }

        for (int i = 0; i < 50; i++) {
            auto * canMessage = new Vector::BLF::CanMessage;
            canMessage->channel = 1;
            canMessage->flags = 1; // TX
            canMessage->dlc = 2;
            canMessage->id = 0x33;
            canMessage->data[0] = 0x44;

            file.write(canMessage);
            std::cout << "Write: " << i << std::endl;
        }

        std::cout << "Written" << std::endl;
        file.close();
        std::cout << "Closed" << std::endl;
    }

    if (rename("test.blf", "firstlog.blf") != 0) {
        std::cout << "New log file failed" << std::endl;
    } else {
        std::cout << "New log file created" << std::endl;
    }

    {
        Vector::BLF::File file;
        file.open("test.blf", std::ios_base::out);

        for (int i = 0; i < 50; i++) {
            auto * canMessage = new Vector::BLF::CanMessage;
            canMessage->channel = 1;
            canMessage->flags = 1; // TX
            canMessage->dlc = 2;
            canMessage->id = 0x33;
            canMessage->data[0] = 0x44;

            file.write(canMessage);
            std::cout << "Write: " << i << std::endl;
        }

        file.close();
        std::cout << "File closed:" << std::endl;
    }
}

Bye,

Tobias