Don't block users with the warning thread. (#1350)

Co-authored-by: Jakub Roztocil <jakub@roztocil.co>

CHANGELOG.md

@@ -8,8 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 - Added support for session persistence of repeated headers with the same name. ([#1335](https://github.com/httpie/httpie/pull/1335))
 - Changed `httpie plugins` to the new `httpie cli` namespace as `httpie cli plugins` (`httpie plugins` continues to work as a hidden alias). ([#1320](https://github.com/httpie/httpie/issues/1320))
 - Fixed redundant creation of `Content-Length` header on `OPTIONS` requests. ([#1310](https://github.com/httpie/httpie/issues/1310))
+- Fixed blocking of warning thread on some use cases. ([#1349](https://github.com/httpie/httpie/issues/1349))
 - Added support for sending `Secure` cookies to the `localhost` (and `.local` suffixed domains). ([#1308](https://github.com/httpie/httpie/issues/1308))
 
+
 ## [3.1.0](https://github.com/httpie/httpie/compare/3.0.2...3.1.0) (2022-03-08)
 
 - **SECURITY** Fixed the [vulnerability](https://github.com/httpie/httpie/security/advisories/GHSA-9w4w-cpc8-h2fq) that caused exposure of cookies on redirects to third party hosts. ([#1312](https://github.com/httpie/httpie/pull/1312))

httpie/uploads.py

@@ -2,7 +2,6 @@ import sys
 import os
 import zlib
 import functools
-import time
 import threading
 from typing import Any, Callable, IO, Iterable, Optional, Tuple, Union, TYPE_CHECKING
 from urllib.parse import urlencode
@@ -110,17 +109,20 @@ def observe_stdin_for_data_thread(env: Environment, file: IO, read_event: thread
         return None
 
     def worker(event: threading.Event) -> None:
-        time.sleep(READ_THRESHOLD)
+        if not event.wait(timeout=READ_THRESHOLD):
-        if not event.is_set():
             env.stderr.write(
                 f'> warning: no stdin data read in {READ_THRESHOLD}s '
                 f'(perhaps you want to --ignore-stdin)\n'
                 f'> See: https://httpie.io/docs/cli/best-practices\n'
             )
 
+    # Making it a daemon ensures that if the user exits from the main program
+    # (e.g. either regularly or with Ctrl-C), the thread will not
+    # block them.
     thread = threading.Thread(
         target=worker,
-        args=(read_event,)
+        args=(read_event,),
+        daemon=True
     )
     thread.start()
 

tests/test_uploads.py

@@ -18,6 +18,8 @@ from .utils import (
 )
 from .fixtures import FILE_PATH_ARG, FILE_PATH, FILE_CONTENT
 
+MAX_RESPONSE_WAIT_TIME = 2
+
 
 def test_chunked_json(httpbin_with_chunked_support):
     r = http(
@@ -90,7 +92,7 @@ def test_chunked_raw(httpbin_with_chunked_support):
 
 
 @contextlib.contextmanager
-def stdin_processes(httpbin, *args):
+def stdin_processes(httpbin, *args, warn_threshold=0.1):
     process_1 = subprocess.Popen(
         [
             "cat"
@@ -110,7 +112,7 @@ def stdin_processes(httpbin, *args):
         stderr=subprocess.PIPE,
         env={
             **os.environ,
-            "HTTPIE_STDIN_READ_WARN_THRESHOLD": "0.1"
+            "HTTPIE_STDIN_READ_WARN_THRESHOLD": str(warn_threshold)
         }
     )
     try:
@@ -132,7 +134,7 @@ def test_reading_from_stdin(httpbin, wait):
             time.sleep(1)
 
         try:
-            _, errs = process_2.communicate(timeout=0.25)
+            _, errs = process_2.communicate(timeout=MAX_RESPONSE_WAIT_TIME)
         except subprocess.TimeoutExpired:
             errs = b''
 
@@ -148,7 +150,7 @@ def test_stdin_read_warning(httpbin):
         process_1.communicate(timeout=0.1, input=b"bleh\n")
 
         try:
-            _, errs = process_2.communicate(timeout=0.25)
+            _, errs = process_2.communicate(timeout=MAX_RESPONSE_WAIT_TIME)
         except subprocess.TimeoutExpired:
             errs = b''
 
@@ -164,13 +166,27 @@ def test_stdin_read_warning_with_quiet(httpbin):
         process_1.communicate(timeout=0.1, input=b"bleh\n")
 
         try:
-            _, errs = process_2.communicate(timeout=0.25)
+            _, errs = process_2.communicate(timeout=MAX_RESPONSE_WAIT_TIME)
         except subprocess.TimeoutExpired:
             errs = b''
 
         assert b'> warning: no stdin data read in 0.1s' not in errs
 
 
+@pytest.mark.requires_external_processes
+@pytest.mark.skipif(is_windows, reason="Windows doesn't support select() calls into files")
+def test_stdin_read_warning_blocking_exit(httpbin):
+    # Use a very large number.
+    with stdin_processes(httpbin, warn_threshold=999) as (process_1, process_2):
+        # Wait before sending any data
+        time.sleep(1)
+        process_1.communicate(timeout=0.1, input=b"some input\n")
+
+        # If anything goes wrong, and the thread starts the block this
+        # will timeout and let us know.
+        process_2.communicate(timeout=MAX_RESPONSE_WAIT_TIME)
+
+
 class TestMultipartFormDataFileUpload:
 
     def test_non_existent_file_raises_parse_error(self, httpbin):