Copilot commented on code in PR #12940:
URL: https://github.com/apache/trafficserver/pull/12940#discussion_r2885912815
##########
tests/gold_tests/thread_config/check_threads.py:
##########
@@ -19,92 +19,130 @@
import psutil
import argparse
+import os
import sys
-
-
-def count_threads(ts_path, etnet_threads, accept_threads, task_threads,
aio_threads):
-
- for p in psutil.process_iter(['name', 'cwd', 'threads']):
-
- # Find the pid corresponding to the ats process we started in autest.
- # It needs to match the process name and the binary path.
- # If autest can expose the pid of the process this is not needed
anymore.
- if p.name() == '[TS_MAIN]' and p.cwd() == ts_path:
-
- etnet_check = set()
- accept_check = set()
- task_check = set()
- aio_check = set()
-
- for t in p.threads():
-
+import time
+
+COUNT_THREAD_WAIT_SECONDS = 10.0
+COUNT_THREAD_POLL_SECONDS = 0.1
+
+
+def _count_threads_once(ts_path, etnet_threads, accept_threads, task_threads,
aio_threads):
+ """
+ Return (code, message) for a single snapshot of ATS thread state.
+ """
+ for p in psutil.process_iter():
+ try:
+ # Find the pid corresponding to the ats process we started in
autest.
+ # It needs to match the process name and the binary path.
+ # If autest can expose the pid of the process this is not needed
anymore.
+ process_name = p.name()
+ process_cwd = p.cwd()
+ process_exe = p.exe()
+ if process_cwd != ts_path:
+ continue
+ if process_name != '[TS_MAIN]' and process_name !=
'traffic_server' and os.path.basename(
+ process_exe) != 'traffic_server':
+ continue
+ except (psutil.NoSuchProcess, psutil.AccessDenied,
psutil.ZombieProcess):
+ continue
+
+ etnet_check = set()
+ accept_check = set()
+ task_check = set()
+ aio_check = set()
+
+ try:
+ threads = p.threads()
+ except (psutil.NoSuchProcess, psutil.AccessDenied,
psutil.ZombieProcess):
+ return 1, 'Could not inspect ATS process threads.'
Review Comment:
`p.threads()` failure returns exit code 1 ("Could not inspect ATS process
threads."), but `count_threads()` treats code 1 as a retryable "process not
found yet" condition. This can hide a real permission/psutil error for up to
the full wait window and makes the exit code ambiguous. Consider using a
distinct non-retry exit code for thread-inspection failures (or exclude this
case from `retry_codes`).
```suggestion
return 12, 'Could not inspect ATS process threads.'
```
##########
tests/gold_tests/thread_config/check_threads.py:
##########
@@ -19,92 +19,130 @@
import psutil
import argparse
+import os
import sys
-
-
-def count_threads(ts_path, etnet_threads, accept_threads, task_threads,
aio_threads):
-
- for p in psutil.process_iter(['name', 'cwd', 'threads']):
-
- # Find the pid corresponding to the ats process we started in autest.
- # It needs to match the process name and the binary path.
- # If autest can expose the pid of the process this is not needed
anymore.
- if p.name() == '[TS_MAIN]' and p.cwd() == ts_path:
-
- etnet_check = set()
- accept_check = set()
- task_check = set()
- aio_check = set()
-
- for t in p.threads():
-
+import time
+
+COUNT_THREAD_WAIT_SECONDS = 10.0
+COUNT_THREAD_POLL_SECONDS = 0.1
+
+
+def _count_threads_once(ts_path, etnet_threads, accept_threads, task_threads,
aio_threads):
+ """
+ Return (code, message) for a single snapshot of ATS thread state.
+ """
+ for p in psutil.process_iter():
+ try:
+ # Find the pid corresponding to the ats process we started in
autest.
+ # It needs to match the process name and the binary path.
+ # If autest can expose the pid of the process this is not needed
anymore.
+ process_name = p.name()
+ process_cwd = p.cwd()
+ process_exe = p.exe()
+ if process_cwd != ts_path:
+ continue
+ if process_name != '[TS_MAIN]' and process_name !=
'traffic_server' and os.path.basename(
+ process_exe) != 'traffic_server':
+ continue
Review Comment:
In the process scan loop, `p.exe()` is fetched before checking whether
`p.cwd()` matches `ts_path`. Since `exe()` can be relatively expensive and can
raise `AccessDenied`, consider only calling `p.exe()` after the `cwd` (and
possibly `name`) filter passes to reduce overhead and avoid unnecessary
permission failures during iteration.
```suggestion
if process_cwd != ts_path:
continue
if process_name != '[TS_MAIN]' and process_name !=
'traffic_server':
process_exe = p.exe()
if os.path.basename(process_exe) != 'traffic_server':
continue
```
##########
tests/gold_tests/thread_config/thread_config.test.py:
##########
@@ -20,6 +20,7 @@
Test.Summary = 'Test that Trafficserver starts with different thread
configurations.'
Test.ContinueOnFail = True
+Test.SkipUnless(Condition.IsPlatform("linux"))
Review Comment:
PR description says the test is skipped on macOS, but this change skips it
on all non-Linux platforms (`SkipUnless(IsPlatform("linux"))`). If the thread
introspection works on other supported OSes (e.g., FreeBSD), consider using a
macOS-specific skip instead, or update the PR description to match the broader
skip behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
