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.
