From maechler at stat.math.ethz.ch  Mon Nov 14 11:05:15 2016
From: maechler at stat.math.ethz.ch (Martin Maechler)
Date: Mon, 14 Nov 2016 11:05:15 +0100
Subject: [Rd] Memory leak with tons of closed connections
In-Reply-To: <CABtg=KnCS0GcP2zYBG2sbj+A0o9N-hdv94Mq5qrCY4xZn7J+AA@mail.gmail.com>
References: <CAPvvxJV3922JWG--M3CPFMu5n4_+nN+CU7umaRKQ8yJPF5t=dw@mail.gmail.com>
	<22565.42645.436216.547440@stat.math.ethz.ch>
	<CAPvvxJU2v9XaxtUp1Nt9W5dEhRLKEvtZQpHfh=cXRMCKkYL8Zg@mail.gmail.com>
	<CABtg=K=FrfpKi67N-ChH6HuuEqY6c-4ymQ9+YCRtbSx8CsUopA@mail.gmail.com>
	<CABtg=KnCS0GcP2zYBG2sbj+A0o9N-hdv94Mq5qrCY4xZn7J+AA@mail.gmail.com>
Message-ID: <22569.35931.449103.667281@stat.math.ethz.ch>

>>>>> G?bor Cs?rdi <csardi.gabor at gmail.com>
>>>>>     on Sun, 13 Nov 2016 20:49:57 +0000 writes:

> Using dup() before fdopen() (and calling fclose() on the connection
> when it is closed) indeed fixes the memory leak.
> 

Thank you, G?bor!
Yes I can confirm that this fixes the memory leak.

I'm testing ('make check-all') currently and then (probably) will
commit the patch.... R-devel only for the time being.

Martin

> FYI,
> Gabor
> 
> Index: src/main/connections.c
> ===================================================================
> --- src/main/connections.c (revision 71653)
> +++ src/main/connections.c (working copy)
> @@ -576,7 +576,7 @@
>      fp = R_fopen(name, con->mode);
>      } else {  /* use file("stdin") to refer to the file and not the console */
>  #ifdef HAVE_FDOPEN
> - fp = fdopen(0, con->mode);
> +        fp = fdopen(dup(0), con->mode);
>  #else
>   warning(_("cannot open file '%s': %s"), name,
>   "fdopen is not supported on this platform");
> @@ -633,8 +633,7 @@
>  static void file_close(Rconnection con)
>  {
>      Rfileconn this = con->private;
> -    if(con->isopen && strcmp(con->description, "stdin"))
> - con->status = fclose(this->fp);
> +    con->status = fclose(this->fp);
>      con->isopen = FALSE;
>  #ifdef Win32
>      if(this->anon_file) unlink(this->name);
> 
> On Fri, Nov 11, 2016 at 1:12 PM, G?bor Cs?rdi <csardi.gabor at gmail.com> wrote:
> > On Fri, Nov 11, 2016 at 12:46 PM, Gergely Dar?czi
> > <daroczig at rapporter.net> wrote:
> > [...]
> >>> I've changed the above to *print* the gc() result every 1000th
> >>> iteration, and after 100'000 iterations, there is still no
> >>> memory increase from the point of view of R itself.
> >
> > Yes, R does not know about it, it does not manage this memory (any
> > more), but the R process requested this memory from the OS, and never
> > gave it back, which is basically the definition of a memory leak. No?
> >
> > I think the leak is because 'stdin' is special and R opens it with fdopen():
> > https://github.com/wch/r-source/blob/f8cdadb769561970cc42776f563043ea5e12fe05/src/main/connections.c#L561-L579
> >
> > and then it does not close it:
> > https://github.com/wch/r-source/blob/f8cdadb769561970cc42776f563043ea5e12fe05/src/main/connections.c#L636
> >
> > I understand that R cannot fclose the FILE*, because that would also
> > close the file descriptor, but anyway, this causes a memory leak. I
> > think.
> >
> > It seems that you cannot close the FILE* without closing the
> > descriptor, so maybe a workaround would be to keep one FILE* open,
> > instead of calling fdopen() to create new ones every time. Another
> > possible workaround is to use dup(), but I don't know enough about the
> > details to be sure.
> >
> > Gabor
> >
> > [...]



[Rd] Memory leak with tons of closed connections
Martin Maechler