On Fri, May 28, 2004 at 10:28:11AM -0400, Mark Phippard wrote:
>
> " As for the pool parameter, the apr_poll() function on the
> iSeries has been enhanced to allow for more descriptors than the
> normal default of 200 defined with FD_SET size. In order to
> support this, the readset, writeset, and exceptset passed to the
> sockets select() function need to be allocated before calling
> select(). The best way to get the space allocated is to allocate
> it from a pool. Using malloc() is a performance problem.
> This pool should be a pool that is unique to the thread calling
> the function, since the pool support in Apache is not threadsafe
> when using the same pool for different threads. The pool in the
> structure passed as the first parameter may or may not be unique
> to the thread, so we could not plan on using that pool. In our
> code, when we use the function, we pass the pool from the request
> rec structure. Since subversion probably doesn't have a request
> object to do that with, this solution probably won't work for you. "
What they're saying there is that the pool that the pfd structure
holds a pointer to might be in use by another thread (in Apache).
I don't know if this is actually true or not, but it is a safe
assumption (if you aren't sure, err on the side of caution).
Whether they were right or not, they added another parameter to
apr_poll, one for the pool that the caller intends any allocations
to be performed in (and guarantees is not in use by any other thread).
>
> The only place that this function is used within Subversion is in file
> marshal.c in libsvn_ra_svn. Here is my current patch, but I think clearly
> based on IBM's description this is not the correct pool to be passing:
>
> Index: subversion/libsvn_ra_svn/marshal.c
> ===================================================================
> --- subversion/libsvn_ra_svn/marshal.c (revision 9884)
> +++ subversion/libsvn_ra_svn/marshal.c (working copy)
> @@ -120,7 +120,11 @@
> }
> pfd.p = pool;
> pfd.reqevents = APR_POLLIN;
> +#ifdef AS400
> + return (APR_STATUS_IS_SUCCESS(apr_poll(&pfd, 1, &n, 0, pool)) && n);
> +#else
> return (APR_STATUS_IS_SUCCESS(apr_poll(&pfd, 1, &n, 0)) && n);
> +#endif
> }
>
The good news is that svnserve uses a distinct tree of pools for each
connection; there should be no cross-thread pool-sharing; IOW, this
change should be safe.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 28 17:36:34 2004