File connot be closed unless all objects have been read

[Former user]

Hello Tobias, I ran into an issue writing a little helper program that cuts down the size of a blf so our tests run faster. It writes the first x number of signals from the provided file into a new file. Hower, the file being read cannot be closed afterwards. The program seems to hang on m_uncompressedFileThread.join(). The only solution I found is reading all remaining objects in the file, although they aren't used for anything. After reading all objects the file can be closed with no problems. I'm running this under Windows 10. Would be great if you could look into it. Thanks a lot, Daniel

#include <iostream>

#include "Vector/BLF.h"

int main(int argc, char *argv[]) {

if (argc != 4)
{
    std::cout << "Invalid number of arguments - must provide:" << std::endl <<
                 "- path of original file from where to read" << std::endl <<
                 "- name of new file to which to write" << std::endl <<
                 "- number of objects to be written" << std::endl <<
                 "Program will read the original file and write the first x number of objects to the new file" << std::endl <<
                 "Example usage: TomiloParserWriteExample.exe original.blf first1000.blf 1000" << std::endl;
    return -1;
}

Vector::BLF::File originalFile;
originalFile.open(argv[1]);
if (!originalFile.is_open())
{
    std::cout << "Unable to open original file" << std::endl;
    return -1;
}

/* open file for writing */
Vector::BLF::File newFile;
newFile.open(argv[2], std::ios_base::out);
if (!newFile.is_open()) {
    std::cout << "Unable to open new file" << std::endl;
    return -1;
}

int max = std::stoi(argv[3]);
int current = 0;
while (originalFile.good() && (current < max))
{
    Vector::BLF::ObjectHeaderBase *ohb = nullptr;

    try
    {
        ohb = originalFile.read();
    }
    catch (std::runtime_error &e)
    {
        std::cout << "Exception: " << e.what() << std::endl;
    }
    if (ohb == nullptr)
        break;

    newFile.write(ohb);

    current++;
}

newFile.close();
// originalFile.close(); DOESN'T WORK -> program hangs on m_uncompressedFileThread.join()

// read remaining objects of file
while (originalFile.good()){
    Vector::BLF::ObjectHeaderBase *ohb = nullptr;

    try
    {
        ohb = originalFile.read();
    }
    catch (std::runtime_error &e)
    {
        std::cout << "Exception: " << e.what() << std::endl;
    }
    if (ohb == nullptr)
        break;

    delete ohb;
}

// closing file now works
originalFile.close();

return 0;

}

[Tobias Lorenz]

Hello Daniel,

I made a change last week in the File::close() method: https://bitbucket.org/tobylorenz/vector_blf/commits/223423a0d2fc6f0be538bba03bf15e4bba0583b8

A customer reported that he has sporadic exceptions with the original implementation and that this change fixes them. I’m wondering if this change now results in the behavior you mentioned. Can you test? Or just test with branch v2.3.

I’m going to implement some further unit tests to check different file closures.

Bye

Tobias

[Daniel Probst]

Hello Tobias,

thanks for the quick response.

I was running the version of your library from about a month ago.

I just checked out the newest version and the commit you linked seems to have solved the issue, since the files can now be closed.

However, some of our tests are now throwing memory leaks.

It seems to me that these are the objects in the ReadWriteQueue that are not getting deleted before closing the file.

Would it be possible for you to check this out.

Thanks, Daniel

‌

==24== 1,234 (1,064 direct, 170 indirect) bytes in 7 blocks are definitely lost in loss record 6 of 8
==24== at 0x4835DEF: operator new(unsigned long) (vg_replace_malloc.c:334)
==24== by 0x4DE6AF7: Vector::BLF::File::createObject(Vector::BLF::ObjectType) (File.cpp:438)
==24== by 0x4DE7D36: Vector::BLF::File::uncompressedFile2ReadWriteQueue() (File.cpp:711)
==24== by 0x4DE8414: Vector::BLF::File::uncompressedFileReadThread(Vector::BLF::File*) (File.cpp:821)
==24== by 0x4DEA631: void std::__invoke_impl<void, void ()(Vector::BLF::File), Vector::BLF::File*>(std::__invoke_other, void (&&)(Vector::BLF::File), Vector::BLF::File*&&) (invoke.h:60)
==24== by 0x4DEA598: std::__invoke_result<void ()(Vector::BLF::File), Vector::BLF::File*>::type std::__invoke<void ()(Vector::BLF::File), Vector::BLF::File*>(void (&&)(Vector::BLF::File), Vector::BLF::File*&&) (invoke.h:95)
==24== by 0x4DEA508: void std::thread::_Invoker<std::tuple<void ()(Vector::BLF::File), Vector::BLF::File*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) (thread:264)
==24== by 0x4DEA4C1: std::thread::_Invoker<std::tuple<void ()(Vector::BLF::File), Vector::BLF::File*> >::operator()() (thread:271)
==24== by 0x4DEA4A5: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void ()(Vector::BLF::File), Vector::BLF::File*> > >::_M_run() (thread:215)
==24== by 0x4F06BEF: ??? (in /usr/local/lib64/libstdc++.so.6.0.28)
==24== by 0x558AFA2: start_thread (pthread_create.c:486)
==24== by 0x529D4CE: clone (clone.S:95)
==24==
==24== 1,626 (352 direct, 1,274 indirect) bytes in 4 blocks are definitely lost in loss record 8 of 8
==24== at 0x4835DEF: operator new(unsigned long) (vg_replace_malloc.c:334)
==24== by 0x4DE6AD9: Vector::BLF::File::createObject(Vector::BLF::ObjectType) (File.cpp:434)
==24== by 0x4DE7D36: Vector::BLF::File::uncompressedFile2ReadWriteQueue() (File.cpp:711)
==24== by 0x4DE8414: Vector::BLF::File::uncompressedFileReadThread(Vector::BLF::File*) (File.cpp:821)
==24== by 0x4DEA631: void std::__invoke_impl<void, void ()(Vector::BLF::File), Vector::BLF::File*>(std::__invoke_other, void (&&)(Vector::BLF::File), Vector::BLF::File*&&) (invoke.h:60)
==24== by 0x4DEA598: std::__invoke_result<void ()(Vector::BLF::File), Vector::BLF::File*>::type std::__invoke<void ()(Vector::BLF::File), Vector::BLF::File*>(void (&&)(Vector::BLF::File), Vector::BLF::File*&&) (invoke.h:95)
==24== by 0x4DEA508: void std::thread::_Invoker<std::tuple<void ()(Vector::BLF::File), Vector::BLF::File*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) (thread:264)
==24== by 0x4DEA4C1: std::thread::_Invoker<std::tuple<void ()(Vector::BLF::File), Vector::BLF::File*> >::operator()() (thread:271)
==24== by 0x4DEA4A5: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void ()(Vector::BLF::File), Vector::BLF::File*> > >::_M_run() (thread:215)
==24== by 0x4F06BEF: ??? (in /usr/local/lib64/libstdc++.so.6.0.28)
==24== by 0x558AFA2: start_thread (pthread_create.c:486)
==24== by 0x529D4CE: clone (clone.S:95)

[Tobias Lorenz]

Hi Daniel,

I’ll check this. Hopefully valgrind shows me the exceptions as well.

The hint is good. I’ll also check the classes, if they correctly delete all objects.

Bye

Tobias

[Daniel Probst]

Hello Tobias,

I was wondering if you have been able to reproduce this yet.

I just ran across what seems to be the same issue again today implementing a new test for CAN messages.

In the file attached you can see a couple of quick tests that I ran to check the problem (using your test_CanMessage.blf file) and the corresponding output of valgrind.

Thanks, Daniel.

[Tobias Lorenz]

Hi Daniel,

yes, I have a test case that hangs when attempting to close the file, while not all objects are read yet.

Thanks for traces. This helps me to find the issue.

At least it seems that not all objects are cleaned up correctly in the destructors.

Bye

Tobias

[Daniel Probst]

Hello Tobias,

after experiencing some more issues with memory leaks, I looked into it and this change to the abort method of the PbjectQueue seems to solve the issue for us.

void ObjectQueue<T>::abort() {
    /* mutex lock */
    std::lock_guard<std::mutex> lock(m_mutex);

    /* stop */
    m_abort = true;

    /* delete elements in queue */
    while(!m_queue.empty()) {
        delete m_queue.front();
        m_queue.pop();
    }

    /* trigger blocked threads */
    tellgChanged.notify_all();
    tellpChanged.notify_all();
}

Could you perhaps test this, and if you see no issues with it, merge it in the master?

Thanks a lot, Daniel

[Tobias Lorenz]

Hi Daniel,

I’m currently fixing the open issues. I totally missed to delete the entries appropriately.

Having this in the abort() function solves the issue. But somehow it feels better to me to have this in the destructor instead. I’ll test the options and fix it.

Bye Tobias