iLCSoft / LCIO

Linear Collider I/O
BSD 3-Clause "New" or "Revised" License
17 stars 34 forks source link

Memory leak for lcio_reader->readNextEvent() #98

Closed mholtrop closed 3 years ago

mholtrop commented 3 years ago

There seems to be a rather hefty memory leak when reading an slcio file the "classic" way with a loop using readNextEvent(), the memory footprint of a very simple code quickly balloons to many GB.

Sample minimal reader below. Note that the slcio files I am reading were created with the v02.07.05 Java version of LCIO. Thanks!

#include <vector>
#include <iostream>

#include <IO/LCReader.h>
#include <IOIMPL/LCFactory.h>

using namespace std;

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

    vector<string> infiles{"/data/HPS/data/physrun2019/slcio/hps_010735_00000.slcio"};

    IO::LCReader* lcio_reader{IOIMPL::LCFactory::getInstance()->createLCReader()};
    EVENT::LCEvent* lcio_event{nullptr};
    int run_number;
    int event_number;

    for(auto file: infiles) {

        cout << "-------------------------------------------------------------------------------\n";
        cout << "LCIO::Open - " << file << "\n\n";
        lcio_reader->open(file);

        bool first_event_read{false};

        while ((lcio_event = lcio_reader->readNextEvent())) {
            run_number = lcio_event->getRunNumber();
            event_number = lcio_event->getEventNumber();

            if(!first_event_read) {
                const vector<string> *col_names = lcio_event->getCollectionNames();
                cout << "Run Number: " << run_number << " first event: " << event_number << "\n\n";
                for( auto col: (*col_names) ){
                    cout << " " << col;
                }
                cout << "\n";
                first_event_read = true;
            }

        }
        cout << "\nRun " << run_number << " last event: " << event_number << "\n\n";
        lcio_reader->close();
    }
}
gaede commented 3 years ago

@mholtrop, @jstrube: something seems to have gone wrong when re-implementing the I/O layer to make LCIO thread-safe. Will look into this.

jstrube commented 3 years ago

Thanks for taking a look. I've tried with 2.14.1, which does have the issue, and with 2.13.3, which does not. I suspect it's a takeCollection which doesn't get cleaned up. I got lost in the different pointer updates in C++14 and C++17, but maybe it's as easy as making this a reference counting pointer, so it gets killed without having to call an explicit delete.

On Mon, Aug 10, 2020 at 10:49 PM Frank Gaede notifications@github.com wrote:

@mholtrop https://github.com/mholtrop, @jstrube https://github.com/jstrube: something seems to have gone wrong when re-implementing the I/O layer to make LCIO thread-safe. Will look into this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iLCSoft/LCIO/issues/98#issuecomment-671740652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACX57QD6G3HU5FVBBIUHQTSADLUZANCNFSM4PYAL77Q .

gaede commented 3 years ago

@jstrube, @mholtrop it should have been fixed with #99 so please use the master for testing. Will make a new release soon.

mholtrop commented 3 years ago

I can confirm that the current master no longer leaks memory like before.

On Aug 12, 2020, at 12:51 AM, Frank Gaede notifications@github.com wrote:

@jstrube https://github.com/jstrube, @mholtrop https://github.com/mholtrop it should have been fixed with #99 https://github.com/iLCSoft/LCIO/pull/99 so please use the master for testing. Will make a new release soon.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iLCSoft/LCIO/issues/98#issuecomment-672571325, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDD7YTDLTY2I7TGU6SODNDSAINU7ANCNFSM4PYAL77Q.