From maechler at stat.math.ethz.ch  Fri Jul 21 17:27:48 2017
From: maechler at stat.math.ethz.ch (Martin Maechler)
Date: Fri, 21 Jul 2017 17:27:48 +0200
Subject: [Rd] [PATCH] Fix memory leak in PicTeXDeviceDriver
In-Reply-To: <5314894.YKaiI5FJ7Q@x2>
References: <5314894.YKaiI5FJ7Q@x2>
Message-ID: <22898.7540.435383.438337@stat.math.ethz.ch>

>>>>> Steve Grubb <sgrubb at redhat.com>
>>>>>     on Thu, 20 Jul 2017 17:06:52 -0400 writes:

    > Hello,
    > This patch fixes a memory leak due to ptd going out of scope
    > before its assigned to dd.

Hmm, I'm not an expert here, but I tend to say 
that it may not be a memory leak because the corresponding
function is static and only called from inside PicTeX() and that has 

    const void *vmax = vmaxget();

at the beginning, and ends with

    vmaxset(vmax);
    return R_NilValue;

maybe because the code writer "saw" that it could not really
free everything correctly ?

The following code - after install.package("sfsmisc")
seems to suggest a very small "leak" but only up to some point, and your
patch does not seem to change much about it:

if(!require("sfsmisc")) install.packages("sfsmisc") # for Sys.sizes()
## but that's not available on Windoze ...

monitor <- function(call, n = 10000, every = n %/% 50, doGC = TRUE) {
    for(i in seq_len(n)) {
        if(i == 1 || i %% every == 0) {
            if(doGC) gc()
            ns <- names(ss <- Sys.sizes())
            cat(sprintf("%5d: (%s=%7d, %s=%7d)\n",
                        i, ns[1], ss[[1]], ns[2], ss[[2]]))
        }
        eval(call)
    }
}

monitor(quote({pictex(); dev.off()}))
##             --------  ^\______needed, otherwise have too many open devices
## goes up to some and then stays

monitor(quote({pdf(); dev.off()}))
##             ---
## goes up to some and then stays ...[almost, sometimes increases still a bit]
## is not much different to pictex()


But I've added a  free(ptd) --- in a way where the code
is slightly easier to parse by a human.

Thank you, Steve!
Martin


    > Signed-off-by: Steve Grubb <sgrubb at redhat.com>

    > Index: src/library/grDevices/src/devPicTeX.c
    > ===================================================================
    > --- src/library/grDevices/src/devPicTeX.c	(revision 72935)
    > +++ src/library/grDevices/src/devPicTeX.c	(working copy)
    > @@ -665,8 +665,10 @@
    > ptd-> width = width;
    > ptd-> height = height;
 
    > -    if( ! PicTeX_Open(dd, ptd) ) 
    > +    if( ! PicTeX_Open(dd, ptd) ) {
    > +        free(ptd);
    > return FALSE;
    > +    }
 
    > /* Base Pointsize */
    > /* Nominal Character Sizes in Pixels */



[Rd] [PATCH] Fix memory leak in PicTeXDeviceDriver
Martin Maechler