Re: [gnet] Memory leak in GConn in GNet 1.4

[David Helder <dhelder umich edu>]

On Sat, 13 Jul 2002, Matt Campbell wrote:
> Suppose a program using GConn allocates a buffer, passes that buffer
> to gnet_conn_write, and loses its pointer to that buffer (as
> echoserver does).  If the write succeeds, then conn->func can free
> the buffer.  If the program needs to call gnet_conn_disconnect or
> gnet_conn_delete before the write has completed or failed, the
> buffer will also be freed, assuming the delete_buffers parameter
> passed to that function is TRUE.

Yes, exactly.

> However, if a write error or timeout occurs, the buffer will not
> be freed until the program exits.  This is so for two reasons:
>
> 1.  conn_write_cb doesn't pass a pointer to the buffer to conn->func
> unless the write succeeded, so conn->func can't free the buffer if
> the write failed.

No, it would call gnet_conn_delete(conn, TRUE) and the buffer would get
deleted.  This is what echoserver does.


> 2.  When write_async_cb and write_async_timeout_cb (in iochannel.c)
> call gnet_io_channel_write_async_cancel, they always pass a false
> delete_buffers parameter to that function.

But they both do an upcall before the call.  That will go to GConn and
then to the user.  The user will call gnet_conn_disconnect or
gnet_conn_delete (which calls gnet_conn_disconnect) with a delete flag,
which calls gnet_io_channel_write_cancel.  So, the buffer will get
deleted.  (Right?)

Actually, there's another bug here.  gnet_io_write_async_cancel is called
twice, so state is freed twice.  I need to add a ref_count to state and
increment it during the upcall.


> One possible solution to this problem would be for conn_write_cb
> to pass a pointer to the buffer to conn->func when the write fails.
> Then, conn->func can check for a non-null buffer parameter if the
> status is GNET_CONN_STATUS_ERROR, and if there is one, free the
> buffer.  There are other solutions as well, but this one is the
> most consistent with the way GConn already works in other cases.
>
> I hope this helps.  If I am wrong about this, please explain why.
> Thanks.

Maybe.  If this works like I think it works and explained, passing a flag
to gnet_conn_delete should work well enough.

David



-- 
      __          _    __ David Helder - dhelder umich edu
  ___/ /__ __  __(_)__/ / <http://www.eecs.umich.edu/~dhelder>
 / _  / _ `/ |/ / / _  /  Jungle Monkey: <http://www.junglemonkey.net>
 |_,_/|_,_/|___/_/|_,_/   Paper CD Case: <http://www.papercdcase.com>