simon-git: putty (master): Simon Tatham

Commits to Tartarus hosted VCS tartarus-commits at lists.tartarus.org
Sun Mar 24 10:22:49 GMT 2019


TL;DR:
  6ecc16fc cryptsuite: clean up exit handling.
  7f9aba63 Handle crashes in the testcrypt binary more cleanly.
  a956da6e cryptsuite: add a general test of ssh_key methods.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: master
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2019-03-24 10:22:49

commit 6ecc16fc4bec36698623091c50c905b88de6dcc9
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=6ecc16fc4bec36698623091c50c905b88de6dcc9;hp=c0e62e97bb9512b88d3e75be7fb444e4236481ca
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Mar 24 09:56:57 2019 +0000

    cryptsuite: clean up exit handling.
    
    Now we only run the final memory-leak check if we didn't already have
    some other error to report, or some other exception that terminated
    the process.
    
    Also, we wait for the subprocess to terminate before returning control
    to the shell, so that any last-minute complaints from Leak Sanitiser
    appear before rather than after the shell prompt comes back.
    
    While I'm here, I've also made check_return_status tolerate the case
    in which the child process never got started at all. That way, if a
    failure manages to occur before even getting _that_ far, there won't
    be a cascade failure from check_return_status getting confused
    afterwards.

 test/cryptsuite.py | 21 ++++++++++++++-------
 test/testcrypt.py  | 15 ++++++++++-----
 2 files changed, 24 insertions(+), 12 deletions(-)

commit 7f9aba638ff61aca4f1be5658577137c819eba0b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=7f9aba638ff61aca4f1be5658577137c819eba0b;hp=6ecc16fc4bec36698623091c50c905b88de6dcc9
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Mar 24 09:33:58 2019 +0000

    Handle crashes in the testcrypt binary more cleanly.
    
    Previously, if the testcrypt subprocess suffered any kind of crash or
    assertion failure during a run of the Python-based test system, the
    effect would be that ChildProcess.read_line() would get EOF, ignore
    it, and silently return the empty string. Then it would carry on doing
    that for the rest of the program, leading to a long string of error
    reports in tests that were nowhere near the code that actually caused
    the crash.
    
    Now ChildProcess.read_line() detects EOF and raises an exception, so
    that the test suite won't heedlessly carry on trying to do things once
    it's noticed that its subprocess has gone away.
    
    This is more fiddly than it sounds, however, because of the wrinkle
    that sometimes that function can be called while a Python __del__
    method is asking testcrypt to free something. If that happens, the
    exception can't be propagated out of the __del__ (analogously to the
    rule that it's a really terrible idea for C++ destructors to throw).
    So you get an annoying warning message on standard error, and then the
    next command sent to testcrypt will be back in the same position.
    Worse still, this can also happen if testcrypt has _already_ crashed,
    because the __del__ methods will still run.
    
    To protect against _that_, ChildProcess caches the exception after
    throwing it, and then each subsequent write_line() will rethrow it.
    And __del__ catches and explicitly ignores the exception (to avoid the
    annoying warning if Python has to do the same).
    
    The combined result should be that if testcrypt crashes in normal
    (non-__del__) context, we should get a single exception that
    terminates the run cleanly without cascade failures, and whose
    backtrace localises the problem to the actual operation that caused
    the crash. If testcrypt crashes in __del__, we can't quite do that
    well, but we can still terminate with an exception at the next
    opportunity, avoiding multiple cascade failures.
    
    Also in this commit, I've got rid of the try-finally in
    cryptsuite.py's (trivial) main program.

 test/testcrypt.py | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

commit a956da6e5b12ef4e91dd3de2aca356c870f7f45b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a956da6e5b12ef4e91dd3de2aca356c870f7f45b;hp=7f9aba638ff61aca4f1be5658577137c819eba0b
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Mar 24 09:16:00 2019 +0000

    cryptsuite: add a general test of ssh_key methods.
    
    This is the test that would have caught the bug described in 867e69187
    if I'd got round to writing it before releasing 0.71. Stable door now
    shut.

 test/cryptsuite.py | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)



More information about the tartarus-commits mailing list