It boils down to a single line of code to show the leak, but I've attached
a simple test program to show it.
Bob.
On Tue, Jul 30, 2013 at 11:27 PM, Thomas Dial <[email protected]> wrote:
> There is a way to write a test for descriptor leaks, and I'd be willing to
> take a crack at it; could the original poster respond to me directly with
> some problematic code?
> Note: I'm relatively unfamiliar with the code base these days, so it may
> take me a little time to get up to speed on how tests are set up.
>
> Regards,
> Tom Dial
> [email protected]
> [email protected]
>
>
>
> ------------------------------
> *From:* Nick Mathewson <[email protected]>
> *To:* [email protected]
> *Sent:* Tuesday, July 30, 2013 8:46 AM
> *Subject:* Re: [Libevent-users] Fwd: leaking munmap
>
> On Tue, Jul 30, 2013 at 1:48 AM, Black Hole <[email protected]>
> wrote:
> >
> >
> > ---------- Forwarded message ----------
> > From: Black Hole <[email protected]>
> > Date: Tue, Jul 30, 2013 at 3:46 PM
> > Subject: leaking munmap
> > To: [email protected]
> >
> >
> > Using 2.1-alpha.
> >
> > In evbuffer_file_segment_free, buffer.c:3077
> >
> > I think the line:
> >
> > if (munmap(seg->mapping, seg->length) == -1)
> >
> > should read:
> >
> > off_t offset_leftover = seg->file_offset & (page_size - 1);
> >
> > if (munmap(seg->mapping, seg->length + offset_leftover) == -1)
> >
> >
> > Without that change, it would leak memory and kernel descriptors.
>
> Hm. Seems plausible; I wonder if there's a way to write a test for this.
>
>
>
> > On a related note, would there be any benefit in caching the pagesize
> > instead of calling sysconf() each time? Something like:
> >>
> >> /* pagesize should be a power of 2 */
> >> long getpagesize_(void) {
> >> static long page_size = -2;
> >> if (page_size == -2) {
> >> #ifdef SC_PAGE_SIZE
> >> page_size = sysconf(SC_PAGE_SIZE);
> >> #elif defined(_SC_PAGE_SIZE)
> >> page_size = sysconf(_SC_PAGE_SIZE);
> >> #else
> >> page_size = 4096;
> >> #endif
> >> }
> >> return page_size;
> >> }
>
> Is there a platform where sysconf is slow or expensive? My
> understanding is that it's usually (always?) a libc function, not a
> syscall, and it should run pretty fast.
>
>
>
> > Also, in evbuffer_file_segment_materialize() around line 2980, the
> > calculation of "offset_leftover" we can remove the modulus in favour of a
> > mask.
> >>
> >> long page_size = getpagesize_();
> >>
> >> if (page_size == -1)
> >>
> >> goto err;
> >>
> >> offset_leftover = offset & (page_size - 1);
>
> Assuming that the page size is always a power of 2, yeah. (I'm not
> aware of any systems pathological enough to have 6K pages or
> something, so this should be fine.)
>
>
> --
> Nick
> ***********************************************************************
> To unsubscribe, send an e-mail to [email protected] with
> unsubscribe libevent-users in the body.
>
>
>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>
#include <errno.h>
#include <event2/buffer.h>
int main(int argc, char *argv[])
{
struct evbuffer_file_segment *seg=NULL;
struct stat st;
int fd = -1;
ev_off_t offset, length;
long pagesize;
char cmd[128], buf[512];
FILE *fp;
if (argc < 2) {
fprintf(stderr, "I need a valid filename\n");
exit(-1);
}
if ((fd = open(argv[1], O_RDONLY)) < 0) {
fprintf(stderr, "open() error: %s\n", strerror(errno));
exit(-1);
}
fstat(fd, &st);
pagesize = sysconf(_SC_PAGE_SIZE);
/* Choose an offset that doesn't fall on a page boundary to trigger the leak. */
offset = 331; /* a random prime that is not a pagesize */
length = pagesize*2; /* not important */
if (offset + length > st.st_size) {
fprintf(stderr, "Pick a file larger than %zd bytes for testing.\n", offset+length);
exit(-1);
}
/*
* THIS IS THE ONLY LINE THAT MATTERS!
* This line should be enough to trigger a 'materialize' which mmap's and leaks during free.
*/
seg = evbuffer_file_segment_new(fd, offset, length, EVBUF_FS_DISABLE_LOCKING | EVBUF_FS_DISABLE_SENDFILE);
/* loose cleanup */
evbuffer_file_segment_free(seg);
close(fd);
/* Run 'lsof' command to show that part of the file is still mapped.
* This could be done on the command line instead of in code,
* or we could use valgrind. */
snprintf(cmd, sizeof(cmd), "/usr/sbin/lsof -nlPp %d", getpid());
if ((fp = popen(cmd, "r")) != NULL) {
while (fgets(buf, sizeof(buf), fp) != NULL) {
buf[sizeof(buf)-1] = 0;
printf(buf);
}
printf("\n");
pclose(fp);
} else {
printf("popen() error: %s\n", strerror(errno));
}
return 0;
}
