Validate that process is actually a patching operation before terminating by michellemcdaniel · Pull Request #367 · Azure/LinuxPatchExtension

michellemcdaniel · 2026-07-23T20:39:48Z

There are times when the previous patching operation will not be running (reboot, etc), where its PID will be reused before the next patching operation starts. When ProcessHandler checks for running previous patching operations, it will identify this new process which is not the patching extension as a process that needs to be terminated. If this process is a required process for the service, it causes that service to crash.

This change checks the process's cmdline by reading the pid's cmdline file in the proc filesystem. If it contains the patching extension Core filename, then we can be more confident that the process we are about to end is the correct one. If it does not find the core filename in the cmdline, we can guarantee that the process is not a patching operation, and we will not end that process.

Logging:

Processes still running from the previous request: [PIDs=[41245]]
Previous patch operation is still in progress with last status update at 2026-07-23T20:10:34Z. Waiting for a maximum of 0:02:59.461797 seconds for it to complete with intermittent status change checks. Next check will be performed after 60 seconds.
Reading file. [File=CoreState.json]
Previous patch operation is still in progress with last status update at 2026-07-23T20:10:34Z. Waiting for a maximum of 0:01:59.403644 seconds for it to complete with intermittent status change checks. Next check will be performed after 60 seconds.
Reading file. [File=CoreState.json]
Previous patch operation is still in progress with last status update at 2026-07-23T20:10:34Z. Waiting for a maximum of 0:00:59.364601 seconds for it to complete with intermittent status change checks. Next check will be performed after 59.364601 seconds.
Reading file. [File=CoreState.json]
Previous request did not complete in time. Terminating all of it's running processes.
DEBUG: Process is a patching operation. [PID=41245]
Terminating process: [PID=41245]
Deleting file. [File=CoreState.json]
DEBUG: Deleting format-matching items from temp folder. [FormatList=[*]][TempFolderLocation=/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.69/tmp]

…ting There are times when the previous patching operation will not be running (reboot, etc), where its PID will be reused before the next patching operation starts. When ProcessHandler checks for running previous patching operations, it will identify this new process which is not the patching extension as a process that needs to be terminated. If this process is a required process for the service, it causes that service to effectively crash. This change checks the process's cmdline by reading the pid's cmdline file in the proc filesystem. If it contains the patching extension Core filename, then we can be more confident that the process we are about to end is the correct one. If it does not find the core filename in the cmdline, we can guarantee that the process is not a patching operation, and we will not end that process.

codecov · 2026-07-23T20:42:42Z

Codecov Report

❌ Patch coverage is 38.88889% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.05%. Comparing base (fc7f336) to head (7ca6ffd).

Files with missing lines	Patch %	Lines
src/extension/src/ProcessHandler.py	15.38%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
- Coverage   94.10%   94.05%   -0.05%     
==========================================
  Files         109      109              
  Lines       20642    20659      +17     
==========================================
+ Hits        19425    19431       +6     
- Misses       1217     1228      +11

Flag	Coverage Δ
python27	`94.05% <38.88%> (-0.05%)`	⬇️
python312	`94.05% <38.88%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR improves safety when terminating leftover “previous patch” processes by verifying the target PID actually belongs to the LinuxPatchExtension core process (via /proc/<pid>/cmdline) before sending SIGTERM, reducing the risk of killing an unrelated process whose PID was reused after reboot.

Changes:

Added is_process_patching_operation() to validate a PID’s command line contains the core filename before termination.
Updated kill_process() to terminate only when the process is both running and identified as a patching operation.
Updated unit tests to account for the new termination gating logic.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/extension/src/ProcessHandler.py	Adds process cmdline validation and gates termination on “running && patching operation”.
src/extension/tests/Test_ProcessHandler.py	Adjusts `kill_process` test setup for the new patching-operation check.

Comments suppressed due to low confidence (1)

src/extension/tests/Test_ProcessHandler.py:152

test_kill_process only covers the case where is_process_patching_operation returns True; the PR’s key behavior change is that non-patching processes must NOT be terminated. Add a negative test that ensures os.kill is not called when is_process_patching_operation returns False.

    def test_kill_process(self):
        # setting mocks
        is_process_running_backup = ProcessHandler.is_process_running
        ProcessHandler.is_process_running = self.mock_is_process_running_to_return_true
        is_process_patching_operation_backup = ProcessHandler.is_process_patching_operation
        ProcessHandler.is_process_patching_operation = self.mock_is_process_patching_operation_to_return_true
        os_kill_backup = os.kill
        os.kill = self.mock_os_kill_to_raise_exception

        # error in terminating process
        pid = 123
        process_handler = ProcessHandler(self.logger, self.env_layer, self.ext_output_status_handler)
        self.assertRaises(OSError, process_handler.kill_process, pid)

        # reseting mocks
        ProcessHandler.is_process_running = is_process_running_backup
        ProcessHandler.is_process_patching_operation = is_process_patching_operation_backup
        os.kill = os_kill_backup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    def mock_is_process_running_to_return_true(self, pid):
        return True

+    def mock_is_process_running_to_return_true(self, pid):
+        return True
+
    def mock_os_kill_to_raise_exception(self, pid, sig):
        raise OSError


+    def is_process_patching_operation(self, pid):
+        try:
+            with self.env_layer.file_system.open("/proc/{0}/cmdline".format(str(pid)), mode="r") as cmdline_file:
+                cmdline = cmdline_file.read()
+                if Constants.CORE_CODE_FILE_NAME in cmdline:
+                    self.logger.log_debug("Process is a patching operation. [PID={0}]".format(str(pid)))
+                    return True
+                else:
+                    self.logger.log_debug("Process is not a patching operation. [PID={0}]".format(str(pid)))
+                    return False
+        except Exception as error:
+            self.logger.log_debug("Error checking if process is a patching operation. [PID={0}] [Error={1}]".format(str(pid), repr(error)))
+            return True  # If we cannot determine, assume it is a patching operation to be safe


Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/extension/src/ProcessHandler.py:206

/proc/<pid>/cmdline is a null-separated byte stream; opening it in text mode can introduce decoding differences between Python 2/3 and locales. Reading it as bytes and comparing against a UTF-8 encoded core filename keeps the check deterministic.

            with self.env_layer.file_system.open("/proc/{0}/cmdline".format(str(pid)), mode="r") as cmdline_file:
                cmdline = cmdline_file.read()
                if Constants.CORE_CODE_FILE_NAME in cmdline:

src/extension/src/ProcessHandler.py:214

Returning True on exceptions can still terminate an unrelated process in the exact scenarios this PR is trying to avoid (e.g., cmdline read fails due to transient /proc races or permissions). If the cmdline cannot be read, treat it as "not a patching operation" so kill_process will skip termination.

        except Exception as error:
            self.logger.log_debug("Error checking if process is a patching operation. [PID={0}] [Error={1}]".format(str(pid), repr(error)))
            return True  # If we cannot determine, assume it is a patching operation to be safe

    def test_kill_process(self):
        # setting mocks
        is_process_running_backup = ProcessHandler.is_process_running
        ProcessHandler.is_process_running = self.mock_is_process_running_to_return_true
+        is_process_patching_operation_backup = ProcessHandler.is_process_patching_operation
+        ProcessHandler.is_process_patching_operation = self.mock_is_process_patching_operation_to_return_true
        os_kill_backup = os.kill
        os.kill = self.mock_os_kill_to_raise_exception

        # error in terminating process
        pid = 123
        process_handler = ProcessHandler(self.logger, self.env_layer, self.ext_output_status_handler)
        self.assertRaises(OSError, process_handler.kill_process, pid)

        # reseting mocks
        ProcessHandler.is_process_running = is_process_running_backup
+        ProcessHandler.is_process_patching_operation = is_process_patching_operation_backup
        os.kill = os_kill_backup


yashnap · 2026-07-23T21:00:38Z

        is_process_running_backup = ProcessHandler.is_process_running
        ProcessHandler.is_process_running = self.mock_is_process_running_to_return_true
+        is_process_patching_operation_backup = ProcessHandler.is_process_patching_operation
+        ProcessHandler.is_process_patching_operation = self.mock_is_process_patching_operation_to_return_true


mock_is_process_patching_operation_to_return_true is not defined
Might have accidently defined mock_is_process_running_to_return_true again.

I fixed it. I noticed it too.

yashnap · 2026-07-23T21:24:25Z

This UT class is a good reference for your new UT (mocks) : https://github.com/Azure/LinuxPatchExtension/blob/master/src/core/tests/Test_LifecycleManagerAzure.py

yashnap · 2026-07-23T21:27:31Z

+    def is_process_patching_operation(self, pid):
+        try:
+            with self.env_layer.file_system.open("/proc/{0}/cmdline".format(str(pid)), mode="r") as cmdline_file:
+                cmdline = cmdline_file.read()


Can you add a comment/share how the output of the read line would look like.

yashnap · 2026-07-23T21:32:07Z

+            with self.env_layer.file_system.open("/proc/{0}/cmdline".format(str(pid)), mode="r") as cmdline_file:
+                cmdline = cmdline_file.read()
+                if Constants.CORE_CODE_FILE_NAME in cmdline:
+                    self.logger.log_debug("Process is a patching operation. [PID={0}]".format(str(pid)))


Since this is machine-specific log, we can use log_verbose instead of log_debug. (used for fleet wide diagnosing)

Ok to change to log_verbose

yashnap · 2026-07-23T21:46:31Z

+                    return True
+                else:
+                    self.logger.log_debug("Process is not a patching operation. [PID={0}]".format(str(pid)))
+                    return False


Similar comment to use log_verbose

Please leave as log_debug and include what it actually shows up as instead of our operation. This demonstrates if this actually fixed or changed any outcome at scale.

michellemcdaniel requested review from kjohn-msft, najams and rane-rajasi as code owners July 23, 2026 20:39

Copilot AI review requested due to automatic review settings July 23, 2026 20:39

Copilot started reviewing on behalf of michellemcdaniel July 23, 2026 20:40 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

fix test

7ca6ffd

Copilot AI review requested due to automatic review settings July 23, 2026 20:56

Copilot started reviewing on behalf of michellemcdaniel July 23, 2026 20:56 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

yashnap reviewed Jul 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Validate that process is actually a patching operation before terminating#367

Validate that process is actually a patching operation before terminating#367
michellemcdaniel wants to merge 2 commits into
masterfrom
dev/michelm/validate-process-is-lpe-before-killing

michellemcdaniel commented Jul 23, 2026

Uh oh!

codecov Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

yashnap Jul 23, 2026

Uh oh!

michellemcdaniel Jul 23, 2026

Uh oh!

yashnap commented Jul 23, 2026 •

edited

Loading

Uh oh!

yashnap Jul 23, 2026

Uh oh!

yashnap Jul 23, 2026

Uh oh!

kjohn-msft Jul 23, 2026

Uh oh!

yashnap Jul 23, 2026

Uh oh!

kjohn-msft Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

michellemcdaniel commented Jul 23, 2026

Uh oh!

codecov Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

yashnap Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

michellemcdaniel Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

yashnap commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yashnap Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

yashnap Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

kjohn-msft Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

yashnap Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

kjohn-msft Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jul 23, 2026 •

edited

Loading

yashnap commented Jul 23, 2026 •

edited

Loading