Closed GoogleCodeExporter closed 9 years ago
This problem has been narrowed down to the behaviour of requestCreate() with
non-well-known methods.
requestCreate() takes a simple pointer to method_t. It will just assign that to
the
new request_t without any reference counting.
In the case of well-known method's, they just point to a static set of
pre-initialised method_t's which are never modified or free'd.
In the case of non-well-known-methods (ie, METHOD_OTHER with the method string
set)
then it's allocated and freed as needed.
The problem is the pointer assignment. The original creator of the method may
then
free the method_t. This'll then be overwritten with other data and be free'ed
later
on. This is just going to cause a huge mess.
The initial fix is to patch requestCreate(). I don't know if this is 100%
correct - I
need to investigate all of the paths leading to requestCreate() and ensure that
the
passed in method_t isn't "given" to the new request_t. Ie, that the caller of
requestCreate() (and their callers) free their method_t reference themselves
later.
Original comment by adrian.c...@gmail.com
on 30 Mar 2010 at 11:49
Attachments:
Ok. Try this patch.
* change requestCreate() to duplicate method_t's, rather than just copy pointers
* change the few places where method_t's are actually parsed (rather than just
using
METHOD_GET) to explicitly deallocate their copy afterwards
This should work correctly for non-standard methods without leaking memory or
overwriting/double-free'ing entries.
Original comment by adrian.c...@gmail.com
on 30 Mar 2010 at 2:59
Attachments:
hm has noted (via lusca-users@) that this patch leaks memory in the real world.
Ok,
time to crack out valgrind and figure out why.
Original comment by adrian.c...@gmail.com
on 1 Apr 2010 at 1:03
There's another method assignment in src/store_vary.c that needs to be turned
into a
urlMethodDup().
Question is whether method's are being leaked because they're replacing previous
methods via assignment, rather than free'ing any previous method. Hm! Good
question!
Original comment by adrian.c...@gmail.com
on 1 Apr 2010 at 2:08
the memory leak is not as big as I suspected, it came along a FreeBSD source
upgrade (STable) which has some serious problems
anyway, lusca use much more memory as before, I could compensate by reducing
cache_mem 750 (before patch) to 400 (after patch) which both lead to about
3.5Gig
mem use at runtime, otherwise it goes swapping a gig extra
Original comment by hm@hm.net.br
on 1 Apr 2010 at 7:59
Ok. Please try this patch instead. This patch does -not- fix the crash; it
instead
logs where I may see the occasional leak in the assignment of method_t pointers.
It will log a message when an existing non-known method pointer (that isn't
NULL) is
copied over with some other method. Copying over known method pointers is wrong
but
won't leak - but copying over unknown method pointers will end up with a
dangling
string buffer containing the unknown method string (ie, the RTSP methods.)
Please apply this patch and tell me if it starts logging stuff. I'd like to
commit it
to HEAD as part of the method_t tidyup that I'm now forced to do. :)
Original comment by adrian.c...@gmail.com
on 1 Apr 2010 at 1:37
Attachments:
We'll look at fixing the memory usage later. It's quite possible I've screwed up
somewhere else. I'll investigate that later.
Original comment by adrian.c...@gmail.com
on 1 Apr 2010 at 1:38
H, if you run the earlier version of Lusca (with the rebuild helper fixes),
does it
go back to using less disk IO?
Original comment by adrian.c...@gmail.com
on 1 Apr 2010 at 1:42
Here's an updated patch to try.
Original comment by adrian.c...@gmail.com
on 2 Apr 2010 at 12:27
Attachments:
Here's a patch to try against LUSCA_HEAD rev r14529.
It implements the specific change in method_t handling when creating new
request_t's
from existing method_t pointers.
Please test this and get back to me!
Original comment by adrian.c...@gmail.com
on 3 Apr 2010 at 5:59
Attachments:
It's been committed to LUSCA_HEAD SVN. I'm now waiting for feedback from hm
from his
live live caches to see whether it's fixed the issue.
Original comment by adrian.c...@gmail.com
on 5 Apr 2010 at 10:32
I guess we can consider this fixed. I have some reasonable servers running
since ..2.diff without any further problem. Maybe premature since this didin't
happened every day but for the moment I think case cleared.
thank's
Original comment by hm@hm.net.br
on 8 Apr 2010 at 6:46
Sweet! Thanks very much.
Original comment by adrian.c...@gmail.com
on 8 Apr 2010 at 7:42
Original issue reported on code.google.com by
hm@hm.net.br
on 30 Mar 2010 at 11:14