Bug 42728 - mod_ssl thread detaching not releasing handles
Summary: mod_ssl thread detaching not releasing handles
Status: NEW
Alias: None
Product: APR
Classification: Unclassified
Component: APR (show other bugs)
Version: HEAD
Hardware: PC Windows Server 2003
: P2 normal with 6 votes (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-23 18:00 UTC by Beau Croteau
Modified: 2008-06-27 05:27 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Beau Croteau 2007-06-23 18:00:03 UTC
the mod_ssl module has the following code implmented:
static unsigned long ssl_util_thr_id(void)
{
    /* OpenSSL needs this to return an unsigned long.  On OS/390, the pthread
     * id is a structure twice that big.  Use the TCB pointer instead as a
     * unique unsigned long.
     */
#ifdef __MVS__
    struct PSA {
        char unmapped[540];
        unsigned long PSATOLD;
    } *psaptr = 0;

    return psaptr->PSATOLD;
#else
    return (unsigned long) apr_os_thread_current();
#endif
}

And when the following code is called inside of openssl 0.9.8e:
	case DLL_THREAD_DETACH:
		ERR_remove_state(0);
		break;

This causes the CRYPTO_set_id_callback() to be called.  The problem is that
apr_os_thread_current() calls DuplicateHandle for the thread and this causes the
thread HANDLE to leak for the detaching thread.

This can be reproduced by having an apache module that that does the following
with the mod_ssl module loaded:
static void* APR_THREAD_FUNC testThread2(apr_thread_t *thd,void* input)
{
	printf("Sample module: Another thread being launching -
%d\n",GetCurrentThreadId());
	apr_thread_exit(thd,APR_SUCCESS);
	return NULL;
}

static void* APR_THREAD_FUNC testThread(apr_thread_t *thd,void* input)
{
	apr_thread_t *thd_arr;
	apr_pool_t *mp;
	apr_threadattr_t *thd_attr;
	
	apr_pool_create(&mp, NULL);
	apr_threadattr_create (&thd_attr, mp);
	apr_threadattr_detach_set (thd_attr, 1);

	while(1)
	{
		printf("Sample module, launching thread.\n");
		//_beginthread(testThread2,0,NULL);
		apr_thread_create(&thd_arr, thd_attr, testThread2, NULL, mp);
		Sleep(5000);
	}
	apr_thread_exit(thd,APR_SUCCESS);
	return NULL;
}
static int helloworld_handler(request_rec *r) {
	apr_thread_t *thd_arr;
	apr_pool_t *mp;
	apr_threadattr_t *thd_attr;

  /* First, some housekeeping. */
  if (!r->handler || strcasecmp(r->handler, "helloworld") != 0) {
    /* r->handler wasn't "helloworld", so it's none of our business */
    return DECLINED;
  }

  if (r->method_number != M_GET) {
    /* We only accept GET and HEAD requests.
     * They are identical for the purposes of a content generator
     * Returning an HTTP error code causes Apache to return an
     * error page (ErrorDocument) to the client.
     */
    return HTTP_METHOD_NOT_ALLOWED;
  }

	/* OK, we're happy with this request, so we'll return the response. */

	ap_set_content_type(r, "text/html");
	ap_rputs("<title>Hello World!</title> .... etc...starting threads...", r);

	apr_pool_create(&mp, NULL);
	apr_threadattr_create (&thd_attr, mp);
	apr_threadattr_detach_set (thd_attr, 1);
	apr_thread_create(&thd_arr, thd_attr, testThread, NULL, mp);


  /* we return OK to indicate that we have successfully processed
   * the request.  No further processing is required.
   */
  return OK;
}
Comment 1 Joe Orton 2007-09-18 03:05:42 UTC
Calling apr_os_thread_current() should not cause a persistent memory leak; if it
does, it's an APR bug.  (I'm not convinced it does, from reading the Win32 code;
it looks like a once-per-thread allocation, and so harmless?)
Comment 2 Beau Croteau 2007-09-19 07:57:41 UTC
Calling apr_os_thread_current() causes a duplication of the Handle, so when the
thread detaches that handle turns in to a dangling pointer.

The problem is the mod_ssl code overwrites the default behavior for
ssl_util_thr_id which, when called in the case mentioned, will leak the handle.

It is not limited to APR thread creation/destruction, it can be any method you
choose to create/destroy threads and the leak will still occur.  If you use the
standard _beginthread API or CreateThread API, the same thing occurs.  It's
because the thread is detaching and there's nothing to clean up the duplicate
handle after it's detached.
Comment 3 Joe Mudd 2008-06-27 05:27:19 UTC
There is a race condition between apr_thread_create() and dummy_worker().  dummy_worker() could start and proceed past the access of thd->td before apr_thread_create() has had a chance to set (*new)->td.  This sets the TLS tls_apr_thread value to NULL.  So when the created thread calls apr_os_thread_current(), the current thread's handle is duplicated and placed in the tls_apr_thread TLS slot.  The tls_apr_thread recorded HANDLE is never closed.

The race condition and when the thread created as detached (attr->detach) as described in Comment 1 and Comment 2, leak a HANDLE if apr_os_thread_current() is called.