Bug 39737 - LogFormat "%{tid}P" reports wrong thread id on Windows
Summary: LogFormat "%{tid}P" reports wrong thread id on Windows
Status: RESOLVED LATER
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_log_config (show other bugs)
Version: 2.2.2
Hardware: PC Windows Server 2003
: P3 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: MassUpdate
Depends on:
Blocks:
 
Reported: 2006-06-06 19:09 UTC by gaodebin
Modified: 2018-11-07 21:10 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description gaodebin 2006-06-06 19:09:26 UTC
It seems that the access log with format "%{tid}P" gives the wrong thread id
on Windows.  I record the thread id in my kernel driver using
PsGetCurrentThreadId(),
which reports a different number than the apache log "%{tid}P".  I also
tried a 3rd party monitoring software (Process Explorer v10.11), which
returns the same result as mine.  "%{pid}P" gives the correct process id though.

Could someone look into this?  Thanks.
Comment 1 gaodebin 2006-06-12 19:41:14 UTC
Just checked the source, and it's the thread handle that's being logged on
Windows.  I don't see why the handle, instead of the id, is being logged, as the
log is to be used outside of httpd, where logging the id makes more sense.  BTW,
%{pid}P logs the process id on Windows.

Anyway, if there is indeed a reason to log the thread handle, please update the
documentation. 
Comment 2 gaodebin 2006-06-13 15:36:42 UTC
Anyone cares to comment on this?

Thread Ids are unique throughout the system, while handles are only unique
within the process.

It's really a pain not getting the thread id from the logs.  All I'm trying to
do is to find the corresponding thread that served the requests.  I can easily
obtain thread ids of all the threads; I can also get a handle to any thread, but
the integer value of that handle I obtain is different from what's been logged
by http, because the handles to the same thread could be different.
Comment 3 William A. Rowe Jr. 2007-12-30 22:26:23 UTC
You are absolutely correct; the thread id and not its handle should be recorded.

I should think calling gettid() (corresponding to the getpid call in
ap_append_pid) where available would have been the "proper" implementation.

Patches welcome (gettid detection for unix is, of course, required).

One ponders why ap_append_tid wasn't implemented ;-)  (Appending, say, nothing
on a non-threaded architecture).
Comment 4 jfclere 2018-07-24 13:07:20 UTC
apr-1.6.x still use GetCurrentThread() so we have an handle not a thread id.
Comment 5 William A. Rowe Jr. 2018-08-30 06:13:23 UTC
apr_os_thread_current returns a handle (OS representation of a manipulatable
thread.) Similarly apr_os_thread_get. That behavior is correct.

mod_log_custom is doing this;

   apr_os_thread_t tid = apr_os_thread_current();

That isn't portable, but OS specific.

core.c is grabbing this for the pid, not using apr;

    return apr_psprintf(p, "%s%s%" APR_PID_T_FMT, string,
                        delim, getpid());

It seems we simply should switch to gettid(), and presume it is portable?

Interesting there is no APR_TID_T_FMT.

Also odd that apr_pid_t doesn't exist, but APR_PID_T_FMT is defined.

Perhaps it is possible to presume sizeof(pid_t) == sizeof(tid_t)?
Comment 6 Yann Ylavic 2018-08-30 09:33:49 UTC
Don't we need something like this?

Index: srclib/apr/strings/apr_snprintf.c
===================================================================
--- srclib/apr/strings/apr_snprintf.c	(revision 1800753)
+++ srclib/apr/strings/apr_snprintf.c	(working copy)
@@ -487,13 +487,21 @@ static char *conv_apr_sockaddr(apr_sockaddr_t *sa,
 static char *conv_os_thread_t(apr_os_thread_t *tid, char *buf_end, apr_size_t *len)
 {
     union {
+#ifndef WIN32
         apr_os_thread_t tid;
+#else
+        DWORD tid;
+#endif
         apr_uint64_t u64;
         apr_uint32_t u32;
     } u;
     int is_negative;
 
+#ifndef WIN32
     u.tid = *tid;
+#else
+    u.tid = GetThreadId(*tid);
+#endif
     switch(sizeof(u.tid)) {
     case sizeof(apr_int32_t):
         return conv_10(u.u32, TRUE, &is_negative, buf_end, len);
--

(similarly for conv_os_thread_t_hex)
Comment 7 William A. Rowe Jr. 2018-11-07 21:10:01 UTC
Please help us to refine our list of open and current defects; this is a mass update of old and inactive Bugzilla reports which reflect user error, already resolved defects, and still-existing defects in httpd.

As repeatedly announced, the Apache HTTP Server Project has discontinued all development and patch review of the 2.2.x series of releases. The final release 2.2.34 was published in July 2017, and no further evaluation of bug reports or security risks will be considered or published for 2.2.x releases. All reports older than 2.4.x have been updated to status RESOLVED/LATER; no further action is expected unless the report still applies to a current version of httpd.

If your report represented a question or confusion about how to use an httpd feature, an unexpected server behavior, problems building or installing httpd, or working with an external component (a third party module, browser etc.) we ask you to start by bringing your question to the User Support and Discussion mailing list, see [https://httpd.apache.org/lists.html#http-users] for details. Include a link to this Bugzilla report for completeness with your question.

If your report was clearly a defect in httpd or a feature request, we ask that you retest using a modern httpd release (2.4.33 or later) released in the past year. If it can be reproduced, please reopen this bug and change the Version field above to the httpd version you have reconfirmed with.

Your help in identifying defects or enhancements still applicable to the current httpd server software release is greatly appreciated.