<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 05/22/2013 11:27 AM, Tom Hughes
wrote:<br>
</div>
<blockquote cite="mid:519D0000.40605@compton.nu" type="cite">On
22/05/13 18:20, Kai Krueger wrote:
<br>
<br>
<blockquote type="cite">On 05/22/2013 11:04 AM, Tom Hughes wrote:
<br>
<br>
<blockquote type="cite">So if a new child is started, and
multiple requests arrive more or
<br>
less simultaneously to different threads in that process, then
they
<br>
will both try and allocate the stores array which means they
will both
<br>
be trying to manipulate the memory pool at the same time.
<br>
</blockquote>
</blockquote>
>
<br>
<blockquote type="cite">The apache routines to manipulate memory
pools should be thread safe, so
<br>
that part should be fine.
<br>
</blockquote>
<br>
That's not what the interwebs are telling me - do you have some
documentation for that claim? Only I'm finding quotes like "Pools
are explicitly thread unsafe".<br>
</blockquote>
<br>
Ouch. Looks like you are right. It sais functions like
apr_pool_create are thread-safe, but those are only the ones to
create new pools, not the general functions.<br>
<br>
<br>
So that needs fixing and probably the rest of mod_tile checked to
see if those functions are used incorrectly anywhere else.<br>
<br>
<br>
I guess the upside is, that that possibly means we have found the
cause and can relatively easily fix it and don't have to go on a
long debuging hunt. Thanks.<span style="color: rgb(37, 53, 85);
font-family: 'Lucida Grande', Verdana, Geneva, Arial, sans-serif;
font-size: 12px; font-style: normal; font-variant: normal;
font-weight: bold; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: nowrap; widows: auto;
word-spacing: 0px; -webkit-text-size-adjust: auto;
-webkit-text-stroke-width: 0px; background-color: rgb(226, 232,
242); display: inline !important; float: none;"></span>
<blockquote cite="mid:519D0000.40605@compton.nu" type="cite">
<br>
That's why you have things like per-request pools - so that you
can do allocations in request context without locking overheads as
well as so you can clean up easily.
<br>
<br>
<blockquote type="cite">It does look like it is possible that
multiple processes can allocate a
<br>
new storage array simultaneously, but that should "only" lead to
memory
<br>
leak, rather than crashes. In that race, simply one of the
threads wins
<br>
and gets to set the stores array and the other allocated arrays
go
<br>
unused. As all allocations are equivalent, it shouldn't matter
which wins.
<br>
<br>
That race should be fixable, by simply adding an explicit lock
after the
<br>
stores==null check. As this only happens at process / thread
<br>
initialisation and all operations are fast, the performance
impact of
<br>
that should be negligible.
<br>
</blockquote>
<br>
Can the stores array not just be allocated in the child_init hook?
<br>
</blockquote>
I can't remember the details, but I believe child_init hook did not
do at all what I wanted. I think it might have again only been per
process and not per thread.<br>
<blockquote cite="mid:519D0000.40605@compton.nu" type="cite">
<br>
That way it is only the apr_pool_userdata_get call that you are
relying on to be thread safe - no idea if it is, but at it is
reading things it is more likely to be.</blockquote>
With the explicit mutex after the stores == null (and the
appropriate recheck), apr_pool_userdata_get would also be the only
function that needs to be thread safe. However, if that is not, then
you would have to put a mutex around that as well. As that is called
on every request that would be less nice having to do that. On the
other hand, given the load we see on typical mod_tile installations,
that shouldn't be an issue either. From the benchmarks I did on the
locking on the stats collection, even at 10k tiles/s the per request
locking didn't seem to have a significant effect.<br>
<br>
I'll try and fix this tonight and hopefully that will then indeed
solve the instability issues Sven and Andy have seen.<br>
<br>
Kai<br>
<blockquote cite="mid:519D0000.40605@compton.nu" type="cite">
<br>
<br>
Tom
<br>
<br>
</blockquote>
<br>
</body>
</html>