Memory leak in select.

Memory leak in select.

[Peter Rosin]

Hi!

Using the following STC, I'm seeing what appears to be a memory
leak in select(2).

----------------8<---(selectleak.c)---------
#include <sys/time.h>
#include <fcntl.h>
#include <stdlib.h>

int
main(void)
{
	fd_set fdset;
	struct timeval tv;

	long flags = fcntl(0, F_GETFL);
	fcntl(0, F_SETFL, flags | O_NONBLOCK);

	for (;;) {
		int res;
		char buf[20];

		FD_ZERO(&fdset);
		FD_SET(0, &fdset);
		tv.tv_sec = 1;
		tv.tv_usec = 0;
		res = select(1, &fdset, NULL, NULL, &tv);
		if (res < 0)
			exit(1);
		if (!res)
			continue;
		res = read(0, buf, sizeof(buf));
		if (res < 0)
			exit(1);
	}

	return 0;
}
----------------8<--------------------------

$ gcc -o selectleak selectleak.c
$ cat /dev/zero | ./selectleak

Note that "./selectleak < /dev/zero" does not trigger the leak for me (I haven't
seen the memory usage go up, so at least not as reliably).

An "strace -m malloc,select" has this:

  156   59281 [main] selectleak 8440 cygwin_select: 1, 0x23CCFC, 0x0, 0x0, 0x0
   64   59345 [main] selectleak 8440 calloc: (1, 44) = 49AF58, called by 61002389
   63   59408 [main] selectleak 8440 calloc: (1, 12) = 49B018, called by 61002389
  111   59519 [main] selectleak 8440 cygwin_select: to NULL, ms FFFFFFFF
   62   59581 [main] selectleak 8440 cygwin_select: sel.always_ready 0
  138   59719 [select_pipe] selectleak 8440 peek_pipe: , ready for read: avail 130892
  160   59879 [main] selectleak 8440 select_stuff::wait: woke up.  wait_ret 1.  verifying
   57   59936 [main] selectleak 8440 set_bits: me 0x49AF58, testing fd 0 ()
   62   59998 [main] selectleak 8440 set_bits: ready 1
   62   60060 [main] selectleak 8440 select_stuff::wait: gotone 1
   61   60121 [main] selectleak 8440 select_stuff::wait: returning 0
   61   60182 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
  104   60286 [main] selectleak 8440 free: (0x49B018), called by 0x610BC1EA
   65   60351 [main] selectleak 8440 peek_pipe: , already ready for read
   62   60413 [main] selectleak 8440 set_bits: me 0x49AF58, testing fd 0 ()
   62   60475 [main] selectleak 8440 set_bits: ready 1
   74   60549 [main] selectleak 8440 select_stuff::poll: returning 1
   61   60610 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
   65   60675 [main] selectleak 8440 select_stuff::~select_stuff: deleting select records
   60   60735 [main] selectleak 8440 free: (0x49AF58), called by 0x610BC2A2
  108   60843 [main] selectleak 8440 calloc: (1, 44) = 49AF58, called by 61002389
   64   60907 [main] selectleak 8440 calloc: (1, 12) = 49B018, called by 61002389
  126   61033 [main] selectleak 8440 peek_pipe: , ready for read: avail 130892
   62   61095 [main] selectleak 8440 fhandler_base::ready_for_read: read_ready 1, avail 1
   62   61157 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
   61   61218 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
   62   61280 [main] selectleak 8440 select_stuff::~select_stuff: deleting select records
   61   61341 [main] selectleak 8440 free: (0x49AF58), called by 0x610BC2A2
  226   61567 [main] selectleak 8440 cygwin_select: 1, 0x23CCFC, 0x0, 0x0, 0x0

Notice how the "calloc: (1, 12) = 49B018" is only freed once between the two
"cygwin_select: 1, 0x23CCFC, ..."-lines.

$ cygcheck -c cygwin gcc4
Cygwin Package Information
Package              Version        Status
cygwin               1.7.9-1        OK
gcc4                 4.3.4-4        OK

Cheers,
Peter

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Peter Rosin]

Den 2011-04-18 13:43 skrev Peter Rosin:
> Hi!
> 
> Using the following STC, I'm seeing what appears to be a memory
> leak in select(2).
> 
> ----------------8<---(selectleak.c)---------
> #include <sys/time.h>
> #include <fcntl.h>
> #include <stdlib.h>
> 
> int
> main(void)
> {
> 	fd_set fdset;
> 	struct timeval tv;
> 
> 	long flags = fcntl(0, F_GETFL);
> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
> 
> 	for (;;) {
> 		int res;
> 		char buf[20];
> 
> 		FD_ZERO(&fdset);
> 		FD_SET(0, &fdset);
> 		tv.tv_sec = 1;
> 		tv.tv_usec = 0;
> 		res = select(1, &fdset, NULL, NULL, &tv);
> 		if (res < 0)
> 			exit(1);
> 		if (!res)
> 			continue;
> 		res = read(0, buf, sizeof(buf));
> 		if (res < 0)
> 			exit(1);
> 	}
> 
> 	return 0;
> }
> ----------------8<--------------------------
> 
> $ gcc -o selectleak selectleak.c
> $ cat /dev/zero | ./selectleak
> 
> Note that "./selectleak < /dev/zero" does not trigger the leak for me (I haven't
> seen the memory usage go up, so at least not as reliably).
> 
> An "strace -m malloc,select" has this:
> 
>   156   59281 [main] selectleak 8440 cygwin_select: 1, 0x23CCFC, 0x0, 0x0, 0x0
>    64   59345 [main] selectleak 8440 calloc: (1, 44) = 49AF58, called by 61002389
>    63   59408 [main] selectleak 8440 calloc: (1, 12) = 49B018, called by 61002389
>   111   59519 [main] selectleak 8440 cygwin_select: to NULL, ms FFFFFFFF
>    62   59581 [main] selectleak 8440 cygwin_select: sel.always_ready 0
>   138   59719 [select_pipe] selectleak 8440 peek_pipe: , ready for read: avail 130892
>   160   59879 [main] selectleak 8440 select_stuff::wait: woke up.  wait_ret 1.  verifying
>    57   59936 [main] selectleak 8440 set_bits: me 0x49AF58, testing fd 0 ()
>    62   59998 [main] selectleak 8440 set_bits: ready 1
>    62   60060 [main] selectleak 8440 select_stuff::wait: gotone 1
>    61   60121 [main] selectleak 8440 select_stuff::wait: returning 0
>    61   60182 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
>   104   60286 [main] selectleak 8440 free: (0x49B018), called by 0x610BC1EA
>    65   60351 [main] selectleak 8440 peek_pipe: , already ready for read
>    62   60413 [main] selectleak 8440 set_bits: me 0x49AF58, testing fd 0 ()
>    62   60475 [main] selectleak 8440 set_bits: ready 1
>    74   60549 [main] selectleak 8440 select_stuff::poll: returning 1
>    61   60610 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
>    65   60675 [main] selectleak 8440 select_stuff::~select_stuff: deleting select records
>    60   60735 [main] selectleak 8440 free: (0x49AF58), called by 0x610BC2A2
>   108   60843 [main] selectleak 8440 calloc: (1, 44) = 49AF58, called by 61002389
>    64   60907 [main] selectleak 8440 calloc: (1, 12) = 49B018, called by 61002389
>   126   61033 [main] selectleak 8440 peek_pipe: , ready for read: avail 130892
>    62   61095 [main] selectleak 8440 fhandler_base::ready_for_read: read_ready 1, avail 1
>    62   61157 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
>    61   61218 [main] selectleak 8440 select_stuff::cleanup: calling cleanup routines
>    62   61280 [main] selectleak 8440 select_stuff::~select_stuff: deleting select records
>    61   61341 [main] selectleak 8440 free: (0x49AF58), called by 0x610BC2A2
>   226   61567 [main] selectleak 8440 cygwin_select: 1, 0x23CCFC, 0x0, 0x0, 0x0
> 
> Notice how the "calloc: (1, 12) = 49B018" is only freed once between the two
> "cygwin_select: 1, 0x23CCFC, ..."-lines.
> 
> $ cygcheck -c cygwin gcc4
> Cygwin Package Information
> Package              Version        Status
> cygwin               1.7.9-1        OK
> gcc4                 4.3.4-4        OK
> 
> Cheers,
> Peter

Arrrg, this is the program I actually used to generate that
strace (but the leak is present in both versions). Oh well,
sorry about that...

----------------8<---(selectleak.c)---------
#include <sys/time.h>
#include <fcntl.h>

int
main(void)
{
	fd_set fdset;

	long flags = fcntl(0, F_GETFL);
	fcntl(0, F_SETFL, flags | O_NONBLOCK);

	for (;;) {
		int res;
		char buf[20];

		FD_ZERO(&fdset);
		FD_SET(0, &fdset);
		res = select(1, &fdset, NULL, NULL, NULL);
		if (!res)
			continue;
		if (res < 0)
			return 1;
		res = read(0, buf, sizeof(buf));
		if (!res)
			break;
		if (res < 0)
			return 1;
	}

	return 0;
}
----------------8<--------------------------

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Peter Rosin]

Den 2011-04-18 14:23 skrev Peter Rosin:
> Den 2011-04-18 13:43 skrev Peter Rosin:
>> Hi!
>>
>> Using the following STC, I'm seeing what appears to be a memory
>> leak in select(2).
>>
> ----------------8<---(selectleak.c)---------
> #include <sys/time.h>
> #include <fcntl.h>
> 
> int
> main(void)
> {
> 	fd_set fdset;
> 
> 	long flags = fcntl(0, F_GETFL);
> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
> 
> 	for (;;) {
> 		int res;
> 		char buf[20];
> 
> 		FD_ZERO(&fdset);
> 		FD_SET(0, &fdset);
> 		res = select(1, &fdset, NULL, NULL, NULL);
> 		if (!res)
> 			continue;
> 		if (res < 0)
> 			return 1;
> 		res = read(0, buf, sizeof(buf));
> 		if (!res)
> 			break;
> 		if (res < 0)
> 			return 1;
> 	}
> 
> 	return 0;
> }
> ----------------8<--------------------------

Ok, I'm taking a wild swing at this, and my guess is that the call
sel.cleanup () in cygwin_select prematurely zeros out the cleanup
member of the select_record. The call to sel.poll () then adds
"stuff" to the select_record that really should have been cleaned
up, but isn't since cleanup has already been executed and then
zapped (by select_stuff::cleanup).

But what do I know?

Cheers,
Peter

extern "C" int
cygwin_select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
               struct timeval *to)
{
  select_stuff sel;
  fd_set *dummy_readfds = allocfd_set (maxfds);
  fd_set *dummy_writefds = allocfd_set (maxfds);
  fd_set *dummy_exceptfds = allocfd_set (maxfds);

  ...

  sel.cleanup ();                          // Too early ???
  copyfd_set (readfds, r, maxfds);
  copyfd_set (writefds, w, maxfds);
  copyfd_set (exceptfds, e, maxfds);
  return timeout ? 0 : sel.poll (readfds, writefds, exceptfds);
}

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Christopher Faylor]

On Mon, Apr 18, 2011 at 04:32:10PM +0200, Peter Rosin wrote:
>Den 2011-04-18 14:23 skrev Peter Rosin:
>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>> Hi!
>>>
>>> Using the following STC, I'm seeing what appears to be a memory
>>> leak in select(2).
>>>
>> ----------------8<---(selectleak.c)---------
>> #include <sys/time.h>
>> #include <fcntl.h>
>> 
>> int
>> main(void)
>> {
>> 	fd_set fdset;
>> 
>> 	long flags = fcntl(0, F_GETFL);
>> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
>> 
>> 	for (;;) {
>> 		int res;
>> 		char buf[20];
>> 
>> 		FD_ZERO(&fdset);
>> 		FD_SET(0, &fdset);
>> 		res = select(1, &fdset, NULL, NULL, NULL);
>> 		if (!res)
>> 			continue;
>> 		if (res < 0)
>> 			return 1;
>> 		res = read(0, buf, sizeof(buf));
>> 		if (!res)
>> 			break;
>> 		if (res < 0)
>> 			return 1;
>> 	}
>> 
>> 	return 0;
>> }
>> ----------------8<--------------------------
>
>Ok, I'm taking a wild swing at this, and my guess is that the call
>sel.cleanup () in cygwin_select prematurely zeros out the cleanup
>member of the select_record. The call to sel.poll () then adds
>"stuff" to the select_record that really should have been cleaned
>up, but isn't since cleanup has already been executed and then
>zapped (by select_stuff::cleanup).
>
>But what do I know?

How does sel.poll add "stuff" that should be cleaned up?  That function
only looks for bits to set.

cgf


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Christopher Faylor]

On Mon, Apr 18, 2011 at 11:24:41AM -0400, Christopher Faylor wrote:
>On Mon, Apr 18, 2011 at 04:32:10PM +0200, Peter Rosin wrote:
>>Den 2011-04-18 14:23 skrev Peter Rosin:
>>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>>> Hi!
>>>>
>>>> Using the following STC, I'm seeing what appears to be a memory
>>>> leak in select(2).
>>>>
>>> ----------------8<---(selectleak.c)---------
>>> #include <sys/time.h>
>>> #include <fcntl.h>
>>> 
>>> int
>>> main(void)
>>> {
>>> 	fd_set fdset;
>>> 
>>> 	long flags = fcntl(0, F_GETFL);
>>> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
>>> 
>>> 	for (;;) {
>>> 		int res;
>>> 		char buf[20];
>>> 
>>> 		FD_ZERO(&fdset);
>>> 		FD_SET(0, &fdset);
>>> 		res = select(1, &fdset, NULL, NULL, NULL);
>>> 		if (!res)
>>> 			continue;
>>> 		if (res < 0)
>>> 			return 1;
>>> 		res = read(0, buf, sizeof(buf));
>>> 		if (!res)
>>> 			break;
>>> 		if (res < 0)
>>> 			return 1;
>>> 	}
>>> 
>>> 	return 0;
>>> }
>>> ----------------8<--------------------------
>>
>>Ok, I'm taking a wild swing at this, and my guess is that the call
>>sel.cleanup () in cygwin_select prematurely zeros out the cleanup
>>member of the select_record. The call to sel.poll () then adds
>>"stuff" to the select_record that really should have been cleaned
>>up, but isn't since cleanup has already been executed and then
>>zapped (by select_stuff::cleanup).
>>
>>But what do I know?
>
>How does sel.poll add "stuff" that should be cleaned up?  That function
>only looks for bits to set.

Also since select() can allocate a persistent thread you can't expect that
the number of allocs will always be equal to the number of frees.  There
could be some allocation of space for thread bookkeeping.

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Peter Rosin]

Den 2011-04-18 17:28 skrev Christopher Faylor:
> On Mon, Apr 18, 2011 at 11:24:41AM -0400, Christopher Faylor wrote:
>> On Mon, Apr 18, 2011 at 04:32:10PM +0200, Peter Rosin wrote:
>>> Den 2011-04-18 14:23 skrev Peter Rosin:
>>>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>>>> Hi!
>>>>>
>>>>> Using the following STC, I'm seeing what appears to be a memory
>>>>> leak in select(2).
>>>>>
>>>> ----------------8<---(selectleak.c)---------
>>>> #include <sys/time.h>
>>>> #include <fcntl.h>
>>>>
>>>> int
>>>> main(void)
>>>> {
>>>> 	fd_set fdset;
>>>>
>>>> 	long flags = fcntl(0, F_GETFL);
>>>> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
>>>>
>>>> 	for (;;) {
>>>> 		int res;
>>>> 		char buf[20];
>>>>
>>>> 		FD_ZERO(&fdset);
>>>> 		FD_SET(0, &fdset);
>>>> 		res = select(1, &fdset, NULL, NULL, NULL);
>>>> 		if (!res)
>>>> 			continue;
>>>> 		if (res < 0)
>>>> 			return 1;
>>>> 		res = read(0, buf, sizeof(buf));
>>>> 		if (!res)
>>>> 			break;
>>>> 		if (res < 0)
>>>> 			return 1;
>>>> 	}
>>>>
>>>> 	return 0;
>>>> }
>>>> ----------------8<--------------------------
>>>
>>> Ok, I'm taking a wild swing at this, and my guess is that the call
>>> sel.cleanup () in cygwin_select prematurely zeros out the cleanup
>>> member of the select_record. The call to sel.poll () then adds
>>> "stuff" to the select_record that really should have been cleaned
>>> up, but isn't since cleanup has already been executed and then
>>> zapped (by select_stuff::cleanup).
>>>
>>> But what do I know?
>>
>> How does sel.poll add "stuff" that should be cleaned up?  That function
>> only looks for bits to set.

I shouldn't have included the strace, and I shouldn't have guessed about
the cause of the problem without verifying my claims. Sorry about that.
But for the record the included strace snippet is reoccurring like that
many many times (the address of the allocation that isn't freed is
moving).  Further evidence; the STC leaks 1 MB every 3 seconds on my
computer, that just can't be right.

I have probably just read the code wrong, and I did say it was a wild
swing. So please ignore my feeble guesses as to what the cause might be
and take it from the symptoms instead. You have been provided with a STC,
and I have certainly done a fair share in cornering the bug my coming up
with that STC. You should be aware that the STC was NOT the first thing
I tried and it was NOT the first thing I thought was wrong when I
observed a leak in one of my "real" programs.

> Also since select() can allocate a persistent thread you can't expect that
> the number of allocs will always be equal to the number of frees.  There
> could be some allocation of space for thread bookkeeping.

Have you tried to invoke the program like I wrote in the original message
(i.e. cat /dev/zero | ./selectleak) before making sweeping statements like
that?

Cheers,
Peter

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Peter Rosin]

Den 2011-04-18 21:23 skrev Peter Rosin:
> Den 2011-04-18 17:28 skrev Christopher Faylor:
>> On Mon, Apr 18, 2011 at 11:24:41AM -0400, Christopher Faylor wrote:
>>> On Mon, Apr 18, 2011 at 04:32:10PM +0200, Peter Rosin wrote:
>>>> Den 2011-04-18 14:23 skrev Peter Rosin:
>>>>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>>>>> Hi!
>>>>>>
>>>>>> Using the following STC, I'm seeing what appears to be a memory
>>>>>> leak in select(2).
>>>>>>
>>>>> ----------------8<---(selectleak.c)---------
>>>>> #include <sys/time.h>
>>>>> #include <fcntl.h>
>>>>>
>>>>> int
>>>>> main(void)
>>>>> {
>>>>> 	fd_set fdset;
>>>>>
>>>>> 	long flags = fcntl(0, F_GETFL);
>>>>> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
>>>>>
>>>>> 	for (;;) {
>>>>> 		int res;
>>>>> 		char buf[20];
>>>>>
>>>>> 		FD_ZERO(&fdset);
>>>>> 		FD_SET(0, &fdset);
>>>>> 		res = select(1, &fdset, NULL, NULL, NULL);
>>>>> 		if (!res)
>>>>> 			continue;
>>>>> 		if (res < 0)
>>>>> 			return 1;
>>>>> 		res = read(0, buf, sizeof(buf));
>>>>> 		if (!res)
>>>>> 			break;
>>>>> 		if (res < 0)
>>>>> 			return 1;
>>>>> 	}
>>>>>
>>>>> 	return 0;
>>>>> }
>>>>> ----------------8<--------------------------
>>>>
>>>> Ok, I'm taking a wild swing at this, and my guess is that the call
>>>> sel.cleanup () in cygwin_select prematurely zeros out the cleanup
>>>> member of the select_record. The call to sel.poll () then adds
>>>> "stuff" to the select_record that really should have been cleaned
>>>> up, but isn't since cleanup has already been executed and then
>>>> zapped (by select_stuff::cleanup).
>>>>
>>>> But what do I know?
>>>
>>> How does sel.poll add "stuff" that should be cleaned up?  That function
>>> only looks for bits to set.
> 
> I shouldn't have included the strace, and I shouldn't have guessed about
> the cause of the problem without verifying my claims. Sorry about that.
> But for the record the included strace snippet is reoccurring like that
> many many times (the address of the allocation that isn't freed is
> moving).  Further evidence; the STC leaks 1 MB every 3 seconds on my
> computer, that just can't be right.

Back with a patch this time.  Fixes it for me...

Cheers,
Peter

2011-04-19  Peter Rosin  <peda@lysator.liu.se>

	* select.cc (pipe_cleanup): Don't leak a select_pipe_info when a
	thread turned out not to be needed.

Index: select.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/select.cc,v
retrieving revision 1.163
diff -u -r1.163 select.cc
--- select.cc	4 Apr 2011 12:23:36 -0000	1.163
+++ select.cc	19 Apr 2011 21:25:44 -0000
@@ -644,13 +644,15 @@
 pipe_cleanup (select_record *, select_stuff *stuff)
 {
   select_pipe_info *pi = (select_pipe_info *) stuff->device_specific_pipe;
-  if (pi && pi->thread)
+  if (!pi)
+    return;
+  if (pi->thread)
     {
       pi->stop_thread = true;
       pi->thread->detach ();
-      delete pi;
-      stuff->device_specific_pipe = NULL;
     }
+  delete pi;
+  stuff->device_specific_pipe = NULL;
 }
 
 int

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Christopher Faylor]

On Tue, Apr 19, 2011 at 11:31:38PM +0200, Peter Rosin wrote:
>Den 2011-04-18 21:23 skrev Peter Rosin:
>> Den 2011-04-18 17:28 skrev Christopher Faylor:
>>> On Mon, Apr 18, 2011 at 11:24:41AM -0400, Christopher Faylor wrote:
>>>> On Mon, Apr 18, 2011 at 04:32:10PM +0200, Peter Rosin wrote:
>>>>> Den 2011-04-18 14:23 skrev Peter Rosin:
>>>>>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>>>>>> Hi!
>>>>>>>
>>>>>>> Using the following STC, I'm seeing what appears to be a memory
>>>>>>> leak in select(2).
>>>>>>>
>>>>>> ----------------8<---(selectleak.c)---------
>>>>>> #include <sys/time.h>
>>>>>> #include <fcntl.h>
>>>>>>
>>>>>> int
>>>>>> main(void)
>>>>>> {
>>>>>> 	fd_set fdset;
>>>>>>
>>>>>> 	long flags = fcntl(0, F_GETFL);
>>>>>> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
>>>>>>
>>>>>> 	for (;;) {
>>>>>> 		int res;
>>>>>> 		char buf[20];
>>>>>>
>>>>>> 		FD_ZERO(&fdset);
>>>>>> 		FD_SET(0, &fdset);
>>>>>> 		res = select(1, &fdset, NULL, NULL, NULL);
>>>>>> 		if (!res)
>>>>>> 			continue;
>>>>>> 		if (res < 0)
>>>>>> 			return 1;
>>>>>> 		res = read(0, buf, sizeof(buf));
>>>>>> 		if (!res)
>>>>>> 			break;
>>>>>> 		if (res < 0)
>>>>>> 			return 1;
>>>>>> 	}
>>>>>>
>>>>>> 	return 0;
>>>>>> }
>>>>>> ----------------8<--------------------------
>>>>>
>>>>> Ok, I'm taking a wild swing at this, and my guess is that the call
>>>>> sel.cleanup () in cygwin_select prematurely zeros out the cleanup
>>>>> member of the select_record. The call to sel.poll () then adds
>>>>> "stuff" to the select_record that really should have been cleaned
>>>>> up, but isn't since cleanup has already been executed and then
>>>>> zapped (by select_stuff::cleanup).
>>>>>
>>>>> But what do I know?
>>>>
>>>> How does sel.poll add "stuff" that should be cleaned up?  That function
>>>> only looks for bits to set.
>> 
>> I shouldn't have included the strace, and I shouldn't have guessed about
>> the cause of the problem without verifying my claims. Sorry about that.
>> But for the record the included strace snippet is reoccurring like that
>> many many times (the address of the allocation that isn't freed is
>> moving).  Further evidence; the STC leaks 1 MB every 3 seconds on my
>> computer, that just can't be right.
>
>Back with a patch this time.  Fixes it for me...
>
>Cheers,
>Peter
>
>2011-04-19  Peter Rosin  <peda@lysator.liu.se>
>
>	* select.cc (pipe_cleanup): Don't leak a select_pipe_info when a
>	thread turned out not to be needed.

Makes sense.  I've checked this in (with a different ChangeLog).

Thanks for the patch.

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Peter Rosin]

Den 2011-04-20 03:10 skrev Christopher Faylor:
> On Tue, Apr 19, 2011 at 11:31:38PM +0200, Peter Rosin wrote:
>>>>>>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>>>>>>> Using the following STC, I'm seeing what appears to be a memory
>>>>>>>> leak in select(2).
>> Back with a patch this time.  Fixes it for me...
>>
>> 2011-04-19  Peter Rosin  <peda@lysator.liu.se>
>>
>> 	* select.cc (pipe_cleanup): Don't leak a select_pipe_info when a
>> 	thread turned out not to be needed.
> 
> Makes sense.  I've checked this in (with a different ChangeLog).
> 
> Thanks for the patch.

Excellent, thanks!

Cheers,
Peter

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Thomas Stalder]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

Hello,

I just have a question, the same change would it not necessary for the
functions: serial_cleanup, mailslot_cleanup and socket_cleanup in
select.cc (like attached patch) ?

I don't know, it's just a question. The patch are not tested.

Regards,

Thomas

2011/4/20 Christopher Faylor
>>2011-04-19  Peter Rosin  <peda@lysator.liu.se>
>>
>>       * select.cc (pipe_cleanup): Don't leak a select_pipe_info when a
>>       thread turned out not to be needed.
>
> Makes sense.  I've checked this in (with a different ChangeLog).

[-- Attachment #2: select.patch --]
[-- Type: application/octet-stream, Size: 1413 bytes --]

--- select.cc_ori	2011-04-20 14:42:03.953125000 +0200
+++ select.cc	2011-04-20 14:45:56.890625000 +0200
@@ -1150,13 +1150,15 @@
 serial_cleanup (select_record *, select_stuff *stuff)
 {
   select_serial_info *si = (select_serial_info *) stuff->device_specific_serial;
-  if (si && si->thread)
+  if (!si)
+    return;
+  if (si->thread)
     {
       si->stop_thread = true;
       si->thread->detach ();
+    }
       delete si;
       stuff->device_specific_serial = NULL;
-    }
 }
 
 select_record *
@@ -1465,15 +1467,17 @@
 {
   select_socket_info *si = (select_socket_info *) stuff->device_specific_socket;
   select_printf ("si %p si->thread %p", si, si ? si->thread : NULL);
-  if (si && si->thread)
+  if (!si)
+    return;
+  if (si->thread)
     {
       SetEvent (si->w4[0]);
       /* Wait for thread to go away */
       si->thread->detach ();
       ResetEvent (si->w4[0]);
+    }
       stuff->device_specific_socket = NULL;
       delete si;
-    }
   select_printf ("returning");
 }
 
@@ -1703,13 +1707,15 @@
 mailslot_cleanup (select_record *, select_stuff *stuff)
 {
   select_mailslot_info *mi = (select_mailslot_info *) stuff->device_specific_mailslot;
-  if (mi && mi->thread)
+  if (!mi)
+    return;
+  if (mi->thread)
     {
       mi->stop_thread = true;
       mi->thread->detach ();
+    }
       delete mi;
       stuff->device_specific_mailslot = NULL;
-    }
 }
 
 select_record *

[-- Attachment #3: Type: text/plain, Size: 218 bytes --]

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Corinna Vinschen]

On Apr 20 15:16, Thomas Stalder wrote:
> Hello,
> 
> I just have a question, the same change would it not necessary for the
> functions: serial_cleanup, mailslot_cleanup and socket_cleanup in
> select.cc (like attached patch) ?
> 
> I don't know, it's just a question. The patch are not tested.

Maybe you should actually test it.  There's a good chance that
it fixes your memory issue.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Thomas Stalder]

Hello,

I have test my patch, but it not resolve my memory issue (
http://cygwin.com/ml/cygwin/2011-04/msg00292.html )

Regards,

Thomas

2011/4/20 Corinna Vinschen
> On Apr 20 15:16, Thomas Stalder wrote:
>> I don't know, it's just a question. The patch are not tested.
>
> Maybe you should actually test it.  There's a good chance that
> it fixes your memory issue.
>
>
> Corinna

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Memory leak in select

[Peter Rosin]

Den 2011-04-18 17:24 skrev Christopher Faylor:
> On Mon, Apr 18, 2011 at 04:32:10PM +0200, Peter Rosin wrote:
>> Den 2011-04-18 14:23 skrev Peter Rosin:
>>> Den 2011-04-18 13:43 skrev Peter Rosin:
>>>> Hi!
>>>>
>>>> Using the following STC, I'm seeing what appears to be a memory
>>>> leak in select(2).
>>>>
>>> ----------------8<---(selectleak.c)---------
>>> #include <sys/time.h>
>>> #include <fcntl.h>
>>>
>>> int
>>> main(void)
>>> {
>>> 	fd_set fdset;
>>>
>>> 	long flags = fcntl(0, F_GETFL);
>>> 	fcntl(0, F_SETFL, flags | O_NONBLOCK);
>>>
>>> 	for (;;) {
>>> 		int res;
>>> 		char buf[20];
>>>
>>> 		FD_ZERO(&fdset);
>>> 		FD_SET(0, &fdset);
>>> 		res = select(1, &fdset, NULL, NULL, NULL);
>>> 		if (!res)
>>> 			continue;
>>> 		if (res < 0)
>>> 			return 1;
>>> 		res = read(0, buf, sizeof(buf));
>>> 		if (!res)
>>> 			break;
>>> 		if (res < 0)
>>> 			return 1;
>>> 	}
>>>
>>> 	return 0;
>>> }
>>> ----------------8<--------------------------
>>
>> Ok, I'm taking a wild swing at this, and my guess is that the call
>> sel.cleanup () in cygwin_select prematurely zeros out the cleanup
>> member of the select_record. The call to sel.poll () then adds
>> "stuff" to the select_record that really should have been cleaned
>> up, but isn't since cleanup has already been executed and then
>> zapped (by select_stuff::cleanup).
>>
>> But what do I know?
> 
> How does sel.poll add "stuff" that should be cleaned up?  That function
> only looks for bits to set.

I added syscall to the strace mask and that places the leak in read(2)
instead. But as stated previously, just look at the symptoms instead and
ignore my finger-pointing.

(I was fooled by the fact that the leak was after the sel.cleanup call,
but my limited experience with strace made me miss the fact that read(2)
was entered before the leak. So, I mistakedly placed the leak in the only
part left in cygwin_select, namely sel.poll)

Cheers,
Peter

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple