Hi guys,
just wanna let you know about a small issue that I ran into.
When you use LAMMPS as a library with the "-in" command-line switch and when you instantiate and destroy many LAMMPS objects within in your "wrapper" program I observed that the input script file is not properly closed unless you use the file() function. If you're aiming to use the one() function instead or just do create-destroy, as is done in the small example program below, you will get an LAMMPS error that the input script cannot be opened b/c of/and an MPI error (at least with open-mpi) that too many files are open.
So, how about adding a line like
if (infile) fclose(infile);
in the destructor of the lammps class or in the destroy() function? That worked for the example program below.
Cheers,
Nils
/* ----- LIKE THIS (COMMENTED CLOSING OF infile OUT)
YOU'RE LIKELY TO GET AN MPI ERROR ------ */
#include "stdio.h"
#include "stdlib.h"
#include "string.h"
#include "mpi.h"
#include "library.h"
#include "lammps.h"
using namespace LAMMPS_NS;
int main(int narg, char **arg) {
int i;
int me, nprocs;
MPI_Init(&narg,&arg);
MPI_Comm_rank(MPI_COMM_WORLD,&me);
MPI_Comm_size(MPI_COMM_WORLD,&nprocs);
for (i=0;i<100000;i++) {
LAMMPS *lmp = new LAMMPS(narg,arg,MPI_COMM_WORLD);
// if (lmp->infile) fclose(lmp->infile);
delete lmp;
}
MPI_Finalize();
return 0;
}
Hi guys,
just wanna let you know about a small issue that I ran into.
When you use LAMMPS as a library with the "-in" command-line switch and
when you instantiate and destroy many LAMMPS objects within in your
"wrapper" program I observed that the input script file is not properly
closed unless you use the file() function. If you're aiming to use the
one() function instead or just do create-destroy, as is done in the
small example program below, you will get an LAMMPS error that the input
script cannot be opened b/c of/and an MPI error (at least with open-mpi)
that too many files are open.
So, how about adding a line like
if (infile) fclose(infile);
in the destructor of the lammps class or in the destroy() function? That
worked for the example program below.
that would not be correct. it would have to be:
if (infile && infile != stdin) fclose(infile);
but i agree, this kind of change should be added to remove the file
descriptor leak.
axel.
True. But I tested it and it all just works if you null the infile pointer in input.cpp after closing the input script file. Otherwise, the addition of
if (infile && infile != stdin) fclose(infile);
into the destructor of the lammps class will give problems if the file() function is used to read in an input script (twice trying to close one and the same file).
Hence, line 184 in input.cpp
if (infile != stdin) fclose(infile);
should be replaced by
if (infile != stdin) {
fclose(infile);
infile=NULL;
}
and the same substitution should be done at line 819.
Cheers,
Nils
True. But I tested it and it all just works if you null the infile pointer
in input.cpp after closing the input script file. Otherwise, the addition of
if (infile && infile != stdin) fclose(infile);
into the destructor of the lammps class will give problems if the file()
function is used to read in an input script (twice trying to close one and
the same file).
Hence, line 184 in input.cpp
if (infile != stdin) fclose(infile);
should be replaced by
if (infile != stdin) {
fclose(infile);
infile=NULL;
}
and the same substitution should be done at line 819.
it is a bit more complex, since you also have to factor in multi-partition runs.
please check out the attached patch relative to the current svn/git
repository code.
i'm also attaching a variant of your test code with a utility function
monitoring the number of open file descriptors (linux-only).
please check it out and let me know, if this works for you, too, or
send me a test that still shows problems. otherwise, i'll forward the
changes to steve for inclusion into the upstream version (LAMMPS-ICMS
will have the changes now).
axel.
lammps-no-fd-leakage.diff.gz (964 Bytes)
test-for-fd-leakage.cpp.gz (497 Bytes)
Thanks, Axel. But I'm good anyway because I put in an explicit closing statement for infile in my actual code, which, in the beginning, tests for usage of "-in" command-line switch to be used. Plus, it doesn't allow for multi-partition runs.