Thanks, Bob. I assume the tests need to compile on all platforms. I know how this will shake out on Linux, but not exactly sure about Windows. Off the cuff, do you think it would be acceptable to get a test set up that (for now) only runs on certain platforms? Maybe that's a question for Nick-- not sure. TD
________________________________ From: Black Hole <[email protected]> To: [email protected] Sent: Tuesday, July 30, 2013 10:42 AM Subject: Re: [Libevent-users] Fwd: leaking munmap 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. > > >
