Sorry for the broken patch. Besides the not so nice multiple registation
of the cleanup, the real problem for the crash after the patch is, that
clear() on a pool already calls the cleanup. So I had to register the
cleanup for the parent pool (pconf) and not for the pool itself.
I'll think about the thread-safety next, but as I said that is not the
cause for your crashes.
Regards,
Rainer
On 13.05.2009 14:56, Henri Gomez wrote:
> Some comments on your latest provided patch :
>
> if (!jk_resolv_pool) {
> if (apr_pool_create(&jk_resolv_pool, (apr_pool_t *)pool)
> != APR_SUCCESS) {
> JK_TRACE_EXIT(l);
> return JK_FALSE;
> }
> }
> /* We need to clear the pool reference, if the pool gets destroyed
> * via its parent pool. */
> apr_pool_cleanup_register(jk_resolv_pool, &jk_resolv_pool,
> jk_resolv_cleanup, jk_resolv_cleanup);
> apr_pool_clear(jk_resolv_pool);
> if (apr_sockaddr_info_get
> (&remote_sa, host, APR_UNSPEC, (apr_port_t) port, 0,
> jk_resolv_pool)
> != APR_SUCCESS) {
> JK_TRACE_EXIT(l);
> return JK_FALSE;
> }
>
> Why not just add the cleanup register in pool create side ?
>
> if (!jk_resolv_pool) {
> if (apr_pool_create(&jk_resolv_pool, (apr_pool_t *)pool)
> != APR_SUCCESS) {
> JK_TRACE_EXIT(l);
> return JK_FALSE;
> }
>
> /* We need to clear the pool reference, if the pool gets destroyed
> * via its parent pool. */
> apr_pool_cleanup_register(jk_resolv_pool, &jk_resolv_pool,
> jk_resolv_cleanup, jk_resolv_cleanup);
> }
>
> apr_pool_clear(jk_resolv_pool);
> if (apr_sockaddr_info_get
> (&remote_sa, host, APR_UNSPEC, (apr_port_t) port, 0,
> jk_resolv_pool)
> != APR_SUCCESS) {
> JK_TRACE_EXIT(l);
> return JK_FALSE;
> }
>
>
> Also what could happen if we get many threads calling jk_resolv at the
> same time ?
Index: common/jk_connect.c
===================================================================
--- common/jk_connect.c (revision 763986)
+++ common/jk_connect.c (working copy)
@@ -35,7 +35,7 @@
#include "apr_errno.h"
#include "apr_general.h"
#include "apr_pools.h"
-static apr_pool_t *jk_apr_pool = NULL;
+static apr_pool_t *jk_resolv_pool = NULL;
#endif
#ifdef HAVE_SYS_FILIO_H
@@ -58,6 +58,13 @@
typedef const char* SET_TYPE;
#endif
+static apr_status_t jk_resolv_cleanup(void *d)
+{
+ /* Clean up pointer content */
+ *(apr_pool_t **)d = NULL;
+ return APR_SUCCESS;
+}
+
/** Set socket to blocking
* @param sd socket to manipulate
* @return errno: fcntl returns -1 (!WIN32)
@@ -343,15 +350,19 @@
apr_sockaddr_t *remote_sa, *temp_sa;
char *remote_ipaddr;
- if (!jk_apr_pool) {
- if (apr_pool_create(&jk_apr_pool, (apr_pool_t *)pool) !=
APR_SUCCESS) {
+ if (!jk_resolv_pool) {
+ if (apr_pool_create(&jk_resolv_pool, (apr_pool_t *)pool) !=
APR_SUCCESS) {
JK_TRACE_EXIT(l);
return JK_FALSE;
}
+ apr_pool_cleanup_register((apr_pool_t *)pool, &jk_resolv_pool,
+ jk_resolv_cleanup, jk_resolv_cleanup);
}
- apr_pool_clear(jk_apr_pool);
+ /* We need to clear the pool reference, if the pool gets destroyed
+ * via its parent pool. */
+ apr_pool_clear(jk_resolv_pool);
if (apr_sockaddr_info_get
- (&remote_sa, host, APR_UNSPEC, (apr_port_t) port, 0, jk_apr_pool)
+ (&remote_sa, host, APR_UNSPEC, (apr_port_t) port, 0,
jk_resolv_pool)
!= APR_SUCCESS) {
JK_TRACE_EXIT(l);
return JK_FALSE;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
