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 ?
You are totally right. I had it in the error branch of the following if,
and then moved it up one to many blocks. Although that makes it more
correct, if the patch doesn't help, the changed patch won't help either,
or did you try? We need to add some logging to the cleanup to see, why
it doesn't work (check, whether it gets called).
> 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 ?
To make it absolutely correct we need to use a mutex. but the crash you
experience is not due to multi-threading issues.
jk_resolv() is called during startup and also later, in case you change
a worker address via the statu worker. The whole startup is done
single-threaded, the init code only runs on one thread.
Changing addresses could be done in multiple threads, if you do it using
parallel requests to the status worker. This is something we should also
fix, but not the problem you observe.
You can set the JkLogLevel to tace, then you will get a log line for
each entry and exit of jk_resolve. You'll notivce, that there will be no
entries without leaving first.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
