[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

serf/ra_serf: issue 2903: crash during cleanup of socket buckets.

From: Lieven Govaerts <svnlgo_at_mobsol.be>
Date: 2007-09-09 21:38:49 CEST

I'm trying to solve svn issue 2903[1] while trying to understand the
solution for serf issue 14[2], which I think causes the svn issue.

This is what happens: at the end of a larger merge over ra_serf,
cleaning up the pools will result in a crash of svn. The source of the
crash is this code in serf/context.c serf_connection_close:
  if (conn->stream != NULL) {
     serf_bucket_destroy(conn->stream);
     conn->stream = NULL;
  }

The conn->stream object is a socket bucket, allocated from an allocator
on the session pool. It's created when the first request on a connection
is going to be sent.

First question: the above code was introduced to fix serf issue 14,
which claims the socket bucket's memory is leaking in some situations.
Why is that? The bucket is allocated on an allocator using pool memory.
So why isn't this bucket destroyed at all times on pool cleanup?

The reason for the crash, is that during pool cleanup the socket bucket
is already destroyed, when serf_connection_close is entered. Above code
tries to destroy the bucket a second time, hence the crash.

Looking at the code in ra_serf, this is what I found as the root cause:

libsvn_ra_serf/serf.c svn_ra_serf__open:
- a 'bucket allocator' is created on the session pool, stored in
serf_sess->bkt_alloc.
- a cleanup handler for the serf_session is registered on the session
pool. It's from this cleanup handler that serf_connection_close is called.
- a new 'bucket allocator' is created on the session pool, stored in
conn->bkt_alloc. It's this allocator that's used to create the socket
bucket.

Now the second allocator is created after the session cleanup handler is
registered, so it's cleaned up before the session cleanup handler is
called (LIFO according to [3]). As a result, the session cleanup handler
will try to delete the socket bucket, which was already be destroyed at
the time the cleanup handler is called.

If I change the code in svn_ra_serf__open to use the first bucket
allocator (the one created before the cleanup handler is registered)
instead of the second, cleanup is handled in the correct order so
there's no crash. Alternatively I could create the second bucket
allocator earlier, before registering the cleanup handler.

Second question: can anyone confirm my analysis of the situation?

Third question: Is there any cleaner way to resolve this issue than to
reorder the creation of allocators and registration of cleanup handlers?

AFAIC the session cleanup handler should not try to destroy the socket
bucket, but that brings me back to my first question. Why?

Lieven

[1]: http://subversion.tigris.org/issues/show_bug.cgi?id=2903
[2]: http://code.google.com/p/serf/issues/detail?id=14&can=1
[3]: http://apr.apache.org/docs/apr/trunk/group___pool_cleanup.html

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Sep 9 21:32:51 2007

This is an archived mail posted to the Subversion Dev mailing list.