apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sander Striker" <s.stri...@striker.nl>
Subject Re: Changing the order of cleanup for some core objects
Date Mon, 21 Jul 2008 09:44:43 GMT
On Mon, Jul 21, 2008 at 9:24 AM, Mladen Turk <mturk@apache.org> wrote:
> Hi,
>
> For you that have track the discussion Bojan, Sander and myself took
> about apr_reslist and pre_cleanup_register this will be familiar.
> For others I'll give a quick overview of the problem and
> API usage diversity. As an example, I'll use the socket API.
>
>
> If you are creating the socket server you'll need at least the
> listening socket and then accept the connections creating more
> sockets. Since the accepted sockets gets created and destroyed
> during the server lifetime, user must ensure the memory doesn't
> leek, by destroying at least the accepted socket memory used.
> Since we use pools, this can be achieved only by creating a separate
> pool for each accepted socked that will be destroyed once the
> socket is not needed any more. All that is pretty straighforward
> and used in many implementations.
>
> Now the problem arises with the order of cleanups.
>
> Socket created with S = apr_socket_create(P) registers its
> cleanup for pool P. The call for apr_socket_close(S) merely
> calls that cleanup causing the underlaying OS socket to get closed.
>
> ...
> S = apr_socket_create(P)
> ... do something
> ... with socket
> apr_socket_close(S) -> calls socket_cleanup(S)
> ...
>
> However if the apr_pool_destroy(P) gets called before
> apr_socket_close call (somebody rise the signal, etc..)
> the apr_pool_destroy call will cause the socket_cleanup(S)
> call and the apr_socket_close(S) will be no-op , and everything
> will behave as expected.
>
> Now, lets assume that socket S accepts connections. To keep
> the memory usage constant the accept loop must clear all the memory
> used so far:
>
> ...
> S = apr_socket_create(P)
> while (alive) {
>  C = apr_pool_create(P)
>  A = apr_socket_accept(S, C)
>  ... do something
>  ... with accepted socket A
>  apr_socket_close(A)
>  apr_pool_destroy(C) // No memory leaks
> }
> apr_socket_close(S)
> ...
>
> So here we create sub-pool (C) of the pool (P)
> and after we've done we destroy that pool to keep
> the memory usage constant.
>
> However if the signal arises (somebody hit CTRL+C)
> the whole sort of problems comes in due to the fact
> that upper algorithm doesn't work any more, and the
> order of objects destruction occurs.

Not seeing that case TBH.  Either the app has a signal handler and
proper teardown logic, or it's a signal you can't catch and the kernel
reclaims all resources anyway.

> For example if we call apr_socket_close(S) while
> the code had A sockets alive, any operation on the
> socket A would fail with some OS specific return value.
>
> ...                    A  = apr_socket_accept(S, C)
> apr_socket_close(S) -> rv = apr_socket_read(A)
> ...                    if (rv == ERROR)
> ...                        apr_socket_close(A)
> ...                    apr_pool_destroy(C)
> ...                    ...
>
> However if instead apr_socket_close the interruption
> was done by apr_pool_destroy(P) (socket S will be
> closed by the cleanup callback) we cannot use the upper
> code any more because:
>
> ...                    A  = apr_socket_accept(S, C)
> apr_pool_destroy(S) -> rv = apr_socket_read(A)
> ...                    if (rv == ERROR)
> ...                        apr_socket_close(A)
> ...                    apr_pool_destroy(C) => !CORE
>
> The reason why the legitimate code cores is because the
> pool from which the socket A was allocated was destroyed
> before the actual cleanup for socket S was called.
>
> The user in this case must figure out the reason of
> the error code from the apr_socket_read and in the
> case the call failed because of the apr_pool_destroy made
> on the parent pool it *must not* call its pool destroy
> because it was destroyed already so you have double-free
> problem.

The fact that the parent pool is being destroyed means that that code
shouldn't be executing anymore in the first place.

> This is usually resolved either by some global variable,
> by additional cleanup for each accepted socket (complete
> waste of resources)

I'll bite, even though it doesn't matter for this discussion:
additional cleanup is not in all cases a complete waste of resources.
Remember, a lot of cleanups fit inside the initial 8k chunk a pool
starts with.

> or something even more innovative ;)
>
> Using pre_cleanup for some objects that presumably have
> child pools can resolve some of those issues, but it
> definitely needs a thorough evaluation.
>
> Comments?

More a question: what is the actual proposal?


Cheers,

Sander

Mime
View raw message