[stable-2.10] pause - do not hang if run in the background (#72065) (#72605)

* Consolidate logic for determining whether or not session is interactive
  into a single function, is_interactive()
* Increase test coverage

I wasn't able to find a good way of simulating running a backgrounded test with CI since the
whole test is essentially run not in a TTY, which is similar enough to cause the new is_interactive()
function to always return false.
(cherry picked from commit 4b8cb6582b)

Co-authored-by: Sam Doran <sdoran@redhat.com>

changelogs/fragments/32143-pause-background-hangs.yml

@@ -0,0 +1,4 @@
+bugfixes:
+  - >
+    pause - Fix indefinite hang when using a pause task on a background
+    process (https://github.com/ansible/ansible/issues/32142)

lib/ansible/plugins/action/pause.py

@@ -24,7 +24,11 @@ import termios
 import time
 import tty
 
-from os import isatty
+from os import (
+    getpgrp,
+    isatty,
+    tcgetpgrp,
+)
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_text, to_native
 from ansible.module_utils.parsing.convert_bool import boolean
@@ -67,6 +71,19 @@ def clear_line(stdout):
     stdout.write(b'\x1b[%s' % CLEAR_TO_EOL)
 
 
+def is_interactive(fd=None):
+    if fd is None:
+        return False
+
+    if isatty(fd):
+        # Compare the current process group to the process group associated
+        # with terminal of the given file descriptor to determine if the process
+        # is running in the background.
+        return getpgrp() == tcgetpgrp(fd)
+    else:
+        return False
+
+
 class ActionModule(ActionBase):
     ''' pauses execution for a length or time, or until input is received '''
 
@@ -177,11 +194,10 @@ class ActionModule(ActionBase):
                 stdout_fd = stdout.fileno()
             except (ValueError, AttributeError):
                 # ValueError: someone is using a closed file descriptor as stdin
-                # AttributeError: someone is using a null file descriptor as stdin on windoez
+                # AttributeError: someone is using a null file descriptor as stdin on windoze
                 stdin = None
-
-            if stdin_fd is not None:
-                if isatty(stdin_fd):
+            interactive = is_interactive(stdin_fd)
+            if interactive:
                 # grab actual Ctrl+C sequence
                 try:
                     intr = termios.tcgetattr(stdin_fd)[6][termios.VINTR]
@@ -214,21 +230,20 @@ class ActionModule(ActionBase):
                 termios.tcflush(stdin, termios.TCIFLUSH)
 
             while True:
+                if not interactive:
+                    display.warning("Not waiting for response to prompt as stdin is not interactive")
+                    if seconds is not None:
+                        # Give the signal handler enough time to timeout
+                        time.sleep(seconds + 1)
+                    break
 
                 try:
-                    if stdin_fd is not None:
-
                     key_pressed = stdin.read(1)
 
                     if key_pressed == intr:  # value for Ctrl+C
                         clear_line(stdout)
                         raise KeyboardInterrupt
 
-                    if not seconds:
-                        if stdin_fd is None or not isatty(stdin_fd):
-                            display.warning("Not waiting for response to prompt as stdin is not interactive")
-                            break
-
                     # read key presses and act accordingly
                     if key_pressed in (b'\r', b'\n'):
                         clear_line(stdout)

test/integration/targets/pause/test-pause-background.yml

@@ -0,0 +1,10 @@
+- name: Test pause in a background task
+  hosts: localhost
+  gather_facts: no
+  become: no
+
+  tasks:
+    - pause:
+
+    - pause:
+        seconds: 1

test/integration/targets/pause/test-pause.yml

@@ -4,6 +4,36 @@
   become: no
 
   tasks:
+    - name: non-integer for duraction (EXPECTED FAILURE)
+      pause:
+        seconds: hello
+      register: result
+      ignore_errors: yes
+
+    - assert:
+        that:
+          - result is failed
+          - "'non-integer' in result.msg"
+
+    - name: non-boolean for echo (EXPECTED FAILURE)
+      pause:
+        echo: hello
+      register: result
+      ignore_errors: yes
+
+    - assert:
+        that:
+          - result is failed
+          - "'not a valid boolean' in result.msg"
+
+    - pause:
+        seconds: 0.1
+      register: results
+
+    - assert:
+        that:
+          - results.stdout is search('Paused for \d+\.\d+ seconds')
+
     - pause:
         seconds: 1
       register: results