Skip to content

Commit

Permalink
Refactor DiffMatchPatch diff handler (#16483)
Browse files Browse the repository at this point in the history
Follow up to #16027
Fixes #15850

Summary of the issue:
Diff Match Patch proxy crashes and the calling thread deadlocks

Description of user facing changes
Diff Match Patch proxy will become more stable

Description of development approach
Refactored DiffMatchPatch diff handler.

Now, when reading from stdout of a proxy process, if not enough bytes are read, the return code is checked.

If a return code was received, an exception is raised and a fallback to difflib occurs.
  • Loading branch information
Danstiv authored May 9, 2024
1 parent ed5ede9 commit 1791eb9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 24 deletions.
59 changes: 35 additions & 24 deletions source/diffHandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,21 @@ def _initialize(self):
def _getText(self, ti: TextInfo) -> str:
return ti.text

@classmethod
def _readData(cls, size: int) -> bytes:
"""Reads from stdout, raises exception on EOF."""
buffer = b""
while (remainingLength := size - len(buffer)) > 0:
chunk = cls._proc.stdout.read(remainingLength)
if chunk:
buffer += chunk
continue
return_code = cls._proc.poll()
if return_code is None:
continue
raise RuntimeError(f'Diff-match-patch proxy process died! Return code {return_code}')
return buffer

def diff(self, newText: str, oldText: str) -> List[str]:
try:
if not newText and not oldText:
Expand All @@ -70,31 +85,23 @@ def diff(self, newText: str, oldText: str) -> List[str]:
return []
with DiffMatchPatch._lock:
self._initialize()
old = oldText.encode("utf-8")
new = newText.encode("utf-8")
oldEncodedText = oldText.encode()
newEncodedText = newText.encode()
# Sizes are packed as 32-bit ints in native byte order.
# Since nvda and nvda_dmp are running on the same Python
# platform/version, this is okay.
tl = struct.pack("=II", len(old), len(new))
DiffMatchPatch._proc.stdin.write(tl)
DiffMatchPatch._proc.stdin.write(old)
DiffMatchPatch._proc.stdin.write(new)
buf = b""
sizeb = b""
SIZELEN = 4
while len(sizeb) < SIZELEN:
try:
sizeb += DiffMatchPatch._proc.stdout.read(SIZELEN - len(sizeb))
except TypeError:
pass
(size,) = struct.unpack("=I", sizeb)
while len(buf) < size:
buf += DiffMatchPatch._proc.stdout.read(size - len(buf))
packedTextLength = struct.pack("=II", len(oldEncodedText), len(newEncodedText))
DiffMatchPatch._proc.stdin.write(packedTextLength)
DiffMatchPatch._proc.stdin.write(oldEncodedText)
DiffMatchPatch._proc.stdin.write(newEncodedText)
DiffMatchPatch._proc.stdin.flush()
DiffMatchPatch._proc.stdout.flush()
DIFF_LENGTH_BUFFER_SIZE = 4
diffLengthBuffer = DiffMatchPatch._readData(DIFF_LENGTH_BUFFER_SIZE)
(diff_length,) = struct.unpack("=I", diffLengthBuffer)
diffBuffer = DiffMatchPatch._readData(diff_length)
return [
line
for line in buf.decode("utf-8").splitlines()
for line in diffBuffer.decode("utf-8").splitlines()
if line and not line.isspace()
]
except Exception:
Expand All @@ -107,11 +114,15 @@ def _terminate(self):
if DiffMatchPatch._proc:
log.debug("Terminating diff-match-patch proxy")
# nvda_dmp exits when it receives two zero-length texts.
try:
DiffMatchPatch._proc.stdin.write(struct.pack("=II", 0, 0))
DiffMatchPatch._proc.wait(timeout=5)
except Exception:
log.exception("Exception during DMP termination")
returnCode = DiffMatchPatch._proc.poll()
if returnCode is None:
try:
DiffMatchPatch._proc.stdin.write(struct.pack("=II", 0, 0))
DiffMatchPatch._proc.wait(timeout=5)
except Exception:
log.exception("Exception during DMP termination")
else:
log.debug(f"Diff-match-patch proxy already terminated, return code is {returnCode}")
DiffMatchPatch._proc = None


Expand Down
1 change: 1 addition & 0 deletions user_docs/en/changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
### Bug Fixes

* NVDA will announce correctly the autocomplete suggestions in Eclipse and other Eclipse-based environments on Windows 11. (#16416, @thgcode)
* Improved reliability of automatic text readout, particularly in terminal applications. (#15850, #16027, @Danstiv)
* Braille cursor routing is now much more reliable when a line contains one or more Unicode variation selectors or decomposed characters. (#10960, #16477, @mltony, @LeonarddeR)
* NVDA will correctly announce selection changes when editing a cell's text in Microsoft Excel. (#15843)

Expand Down

0 comments on commit 1791eb9

Please sign in to comment.