[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [libmicrohttpd] another memory leak .. ;-)
From: |
Christian Grothoff |
Subject: |
Re: [libmicrohttpd] another memory leak .. ;-) |
Date: |
Tue, 9 Aug 2011 09:26:31 +0200 |
User-agent: |
KMail/1.13.7 (Linux/2.6.39-1-amd64; KDE/4.6.5; x86_64; ; ) |
Dear Piotr,
I don't see how this fixes a memory leak, but it would seem to be incorrect as
you repeatedly (without select/poll in the middle) calling gnutls_handshake
may cause blocking IO, which is not acceptable. The code will already re-try.
Are you trying to bound the number of times we call gnutls_handshake (hence
the addition of attempted_number) so that the connection will go away
eventually? That's a hackish way to solve this as TLS may just take longer
than you expect to establish the encrypted connection (i.e. TLS handshake
happens to use a stuttering TCP which generates 128 1-byte messages; as each
is processed individually, this might cause more than 128 times
GNUTLS_E_AGAIN).
The correct approach (to avoid what might seem to be a leak to you, but
technically is not since I don't see any memory truly being leaked) is likely
for you to add a timeout in your call to MHD_daemon_start (so that connections
that are "idle" are eventually removed). But that depends on what the exact
behavior of your 'leaky' application is, your e-mail does not give me enough
context here.
So, I may be wrong here (does the SSL connection go idle? what does the leak
really look like?), but I'd like to see a testcase demonstrating the issue
first before I hack up anything. That way, I can be sure about what is going
on. Right now, the patch just seems to add more problems and I'm not
convinced it solves any.
Happy hacking
Christian
On Tuesday, August 09, 2011 08:16:21 AM you wrote:
> hullo!
>
> hows life back there?
> there exists a significant memory leak in libmicrohttpd. it occurs when
> a https client connects and triggers errors during tls handshake, by
> sending strange data, etc.
> i am enclosing a patch that removes the leak, please take a look at it,
> probably the retry should be done on any !gnutls_error_is_fatal(ret)
> however i am not sure if there are any non-fatal errors besides
> "interrupted" and "again". and on fatal error i think we should close the
> connection (every project i know that uses gnutls closes the connection
> when
> fatal error during handshake occurs).
> i am not sure about the acctual nature of a leak, but it seems that when
> a fatal error occurs, like GNUTLS_E_INTERNAL_ERROR,
> or GNUTLS_E_UNEXPECTED_PACKET, of which i have seen
> a couple, we are not freeing tls context correctly. anyway, please
> consider the patch.
>
> cheers,
> pg
[original patch attached for the mailinglist]
libmicrohttpd_tls_handshake.patch
Description: Text Data
- Re: [libmicrohttpd] another memory leak .. ;-),
Christian Grothoff <=