[Updated] D11087: sigpipe-remote: simply delegate pipe forwarding to subprocess we can kill
marmoute (Pierre-Yves David)
phabricator at mercurial-scm.org
Fri Jul 16 05:05:14 UTC 2021
Closed by commit rHG27ff81547d35: sigpipe-remote: simply delegate pipe forwarding to subprocess we can kill (authored by marmoute).
This revision was automatically updated to reflect the committed changes.
CHANGED PRIOR TO COMMIT
https://phab.mercurial-scm.org/D11087?vs=29205&id=29301#toc
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D11087?vs=29205&id=29301
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D11087/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D11087
AFFECTED FILES
tests/test-transaction-rollback-on-sigpipe.t
tests/testlib/sigpipe-remote.py
tests/testlib/sigpipe-worker.py
CHANGE DETAILS
diff --git a/tests/testlib/sigpipe-worker.py b/tests/testlib/sigpipe-worker.py
new file mode 100755
--- /dev/null
+++ b/tests/testlib/sigpipe-worker.py
@@ -0,0 +1,19 @@
+#!/usr/bin/env python3
+#
+# This is literally `cat` but in python, one char at a time.
+#
+# see sigpipe-remote.py for details.
+from __future__ import print_function
+
+import io
+import os
+import sys
+
+
+if isinstance(sys.stdout.buffer, io.BufferedWriter):
+ print('SIGPIPE-WORKER: script need unbuffered output', file=sys.stderr)
+ sys.exit(255)
+
+while True:
+ c = os.read(sys.stdin.fileno(), 1)
+ os.write(sys.stdout.fileno(), c)
diff --git a/tests/testlib/sigpipe-remote.py b/tests/testlib/sigpipe-remote.py
--- a/tests/testlib/sigpipe-remote.py
+++ b/tests/testlib/sigpipe-remote.py
@@ -5,7 +5,6 @@
import os
import subprocess
import sys
-import threading
import time
# we cannot use mercurial.testing as long as python2 is not dropped as the test will only install the mercurial module for python2 in python2 run
@@ -68,14 +67,6 @@
return s.decode('latin-1')
-piped_stdout = os.pipe2(os.O_NONBLOCK | os.O_CLOEXEC)
-piped_stderr = os.pipe2(os.O_NONBLOCK | os.O_CLOEXEC)
-
-stdout_writer = os.fdopen(piped_stdout[1], "rb")
-stdout_reader = os.fdopen(piped_stdout[0], "rb")
-stderr_writer = os.fdopen(piped_stderr[1], "rb")
-stderr_reader = os.fdopen(piped_stderr[0], "rb")
-
debug_stream.write(b'SIGPIPE-HELPER: Starting\n')
TESTLIB_DIR = os.path.dirname(sys.argv[0])
@@ -88,67 +79,44 @@
SYNCFILE1,
)
-cmd = ['hg']
-cmd += sys.argv[1:]
-sub = subprocess.Popen(
- cmd,
- bufsize=0,
- close_fds=True,
- stdin=sys.stdin,
- stdout=stdout_writer,
- stderr=stderr_writer,
-)
-
-debug_stream.write(b'SIGPIPE-HELPER: Mercurial started\n')
+try:
+ cmd = ['hg']
+ cmd += sys.argv[1:]
+ sub = subprocess.Popen(
+ cmd,
+ bufsize=0,
+ close_fds=True,
+ stdin=sys.stdin,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ )
-
-shut_down = threading.Event()
-
-close_lock = threading.Lock()
-
+ basedir = os.path.dirname(sys.argv[0])
+ worker = os.path.join(basedir, 'sigpipe-worker.py')
-def _read(stream):
- try:
- return stream.read()
- except ValueError:
- # read on closed file
- return None
-
+ cmd = [sys.executable, worker]
-def forward_stdout():
- while not shut_down.is_set():
- c = _read(stdout_reader)
- while c is not None:
- sys.stdout.buffer.write(c)
- c = _read(stdout_reader)
- time.sleep(0.001)
- with close_lock:
- if not stdout_reader.closed:
- stdout_reader.close()
- debug_stream.write(b'SIGPIPE-HELPER: stdout closed\n')
-
+ stdout_worker = subprocess.Popen(
+ cmd,
+ bufsize=0,
+ close_fds=True,
+ stdin=sub.stdout,
+ stdout=sys.stdout,
+ stderr=sys.stderr,
+ )
-def forward_stderr():
- while not shut_down.is_set():
- c = _read(stderr_reader)
- if c is not None:
- sys.stderr.buffer.write(c)
- c = _read(stderr_reader)
- time.sleep(0.001)
- with close_lock:
- if not stderr_reader.closed:
- stderr_reader.close()
- debug_stream.write(b'SIGPIPE-HELPER: stderr closed\n')
-
-
-stdout_thread = threading.Thread(target=forward_stdout, daemon=True)
-stderr_thread = threading.Thread(target=forward_stderr, daemon=True)
-
-try:
- stdout_thread.start()
- stderr_thread.start()
-
+ stderr_worker = subprocess.Popen(
+ cmd,
+ bufsize=0,
+ close_fds=True,
+ stdin=sub.stderr,
+ stdout=sys.stderr,
+ stderr=sys.stderr,
+ )
debug_stream.write(b'SIGPIPE-HELPER: Redirection in place\n')
+ os.close(sub.stdout.fileno())
+ os.close(sub.stderr.fileno())
+ debug_stream.write(b'SIGPIPE-HELPER: pipes closed in main\n')
try:
wait_file(sysbytes(SYNCFILE1))
@@ -157,18 +125,16 @@
debug_stream.write(b'SIGPIPE-HELPER: wait failed: %s\n' % msg)
else:
debug_stream.write(b'SIGPIPE-HELPER: SYNCFILE1 detected\n')
- with close_lock:
- if not stdout_reader.closed:
- stdout_reader.close()
- if not stderr_reader.closed:
- stderr_reader.close()
- sys.stdin.close()
- debug_stream.write(b'SIGPIPE-HELPER: pipes closed\n')
+ stdout_worker.kill()
+ stderr_worker.kill()
+ stdout_worker.wait(10)
+ stderr_worker.wait(10)
+ debug_stream.write(b'SIGPIPE-HELPER: worker killed\n')
+
debug_stream.write(b'SIGPIPE-HELPER: creating SYNCFILE2\n')
write_file(sysbytes(SYNCFILE2))
finally:
debug_stream.write(b'SIGPIPE-HELPER: Shutting down\n')
- shut_down.set()
if not sys.stdin.closed:
sys.stdin.close()
try:
@@ -176,6 +142,11 @@
except subprocess.TimeoutExpired:
msg = b'SIGPIPE-HELPER: Server process failed to terminate\n'
debug_stream.write(msg)
+ sub.kill()
+ sub.wait()
+ msg = b'SIGPIPE-HELPER: Server process killed\n'
else:
- debug_stream.write(b'SIGPIPE-HELPER: Server process terminated\n')
+ msg = b'SIGPIPE-HELPER: Server process terminated with status %d\n'
+ msg %= sub.returncode
+ debug_stream.write(msg)
debug_stream.write(b'SIGPIPE-HELPER: Shut down\n')
diff --git a/tests/test-transaction-rollback-on-sigpipe.t b/tests/test-transaction-rollback-on-sigpipe.t
--- a/tests/test-transaction-rollback-on-sigpipe.t
+++ b/tests/test-transaction-rollback-on-sigpipe.t
@@ -26,37 +26,55 @@
> [hooks]
> pretxnchangegroup.00-break-things=sh "$RUNTESTDIR/testlib/wait-on-file" 10 "$SYNCFILE2" "$SYNCFILE1"
> pretxnchangegroup.01-output-things=echo "some remote output to be forward to the closed pipe"
+ > pretxnchangegroup.02-output-things=echo "some more remote output"
> EOF
$ hg --cwd ./remote tip -T '{node|short}\n'
000000000000
$ cd local
$ echo foo > foo ; hg commit -qAm "commit"
- $ hg push -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" --remotecmd "$remotecmd"
- pushing to ssh://user@dummy/$TESTTMP/remote
- searching for changes
- remote: adding changesets (py3 !)
- remote: adding manifests (py3 !)
- remote: adding file changes (py3 !)
- remote: adding changesets (no-py3 no-chg !)
- remote: adding manifests (no-py3 no-chg !)
- remote: adding file changes (no-py3 no-chg !)
+
+(use quiet to avoid flacky output from the server)
+
+ $ hg push --quiet -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" --remotecmd "$remotecmd"
abort: stream ended unexpectedly (got 0 bytes, expected 4)
[255]
$ cat $SIGPIPE_REMOTE_DEBUG_FILE
SIGPIPE-HELPER: Starting
- SIGPIPE-HELPER: Mercurial started
SIGPIPE-HELPER: Redirection in place
+ SIGPIPE-HELPER: pipes closed in main
SIGPIPE-HELPER: SYNCFILE1 detected
- SIGPIPE-HELPER: pipes closed
+ SIGPIPE-HELPER: worker killed
SIGPIPE-HELPER: creating SYNCFILE2
SIGPIPE-HELPER: Shutting down
- SIGPIPE-HELPER: Server process terminated
+ SIGPIPE-HELPER: Server process terminated with status 255 (no-windows !)
+ SIGPIPE-HELPER: Server process terminated with status 1 (windows !)
SIGPIPE-HELPER: Shut down
The remote should be left in a good state
$ hg --cwd ../remote tip -T '{node|short}\n'
000000000000
+
+#if windows
+
+XXX-Windows Broken behavior to be fixed
+
+Behavior on Windows is broken and should be fixed. However this is a fairly
+corner case situation and no data are being corrupted. This would affect
+central repository being hosted on a Windows machine and accessed using ssh.
+
+This was catch as we setup new CI for Windows. Making the test pass on Windows
+was enough of a pain that fixing the behavior set aside for now. Dear and
+honorable reader, feel free to fix it.
+
+ $ hg --cwd ../remote recover
+ rolling back interrupted transaction
+ (verify step skipped, run `hg verify` to check your repository content)
+
+#else
+
$ hg --cwd ../remote recover
no interrupted transaction available
[1]
+
+#endif
To: marmoute, #hg-reviewers, Alphare
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210716/bd6d9a84/attachment-0002.html>
More information about the Mercurial-patches
mailing list