TestArbitraryPackageAttack TearDownClass failure

[jku]

From PR #1113 CI run:

======================================================================
70ERROR: tearDownClass (test_arbitrary_package_attack.TestArbitraryPackageAttack)
71----------------------------------------------------------------------
72Traceback (most recent call last):
73  File "C:\projects\tuf\tests\test_arbitrary_package_attack.py", line 101, in tearDownClass
74    shutil.rmtree(cls.temporary_directory)
75  File "C:\Python38\lib\shutil.py", line 730, in rmtree
76    return _rmtree_unsafe(path, onerror)
77  File "C:\Python38\lib\shutil.py", line 603, in _rmtree_unsafe
78    _rmtree_unsafe(fullname, onerror)
79  File "C:\Python38\lib\shutil.py", line 603, in _rmtree_unsafe
80    _rmtree_unsafe(fullname, onerror)
81  File "C:\Python38\lib\shutil.py", line 603, in _rmtree_unsafe
82    _rmtree_unsafe(fullname, onerror)
83  File "C:\Python38\lib\shutil.py", line 608, in _rmtree_unsafe
84    onerror(os.unlink, fullname, sys.exc_info())
85  File "C:\Python38\lib\shutil.py", line 606, in _rmtree_unsafe
86    os.unlink(fullname)
87PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\projects\\tuf\\tests\\tmpmsjfh32h\\TestArbitraryPackageAttack_axb40vsy\\repository\\targets\\file1.txt'

Looking at the code that seems logical:

  @classmethod
  def tearDownClass(cls):
    # tearDownModule() is called after all the test cases have run.
    # http://docs.python.org/2/library/unittest.html#class-and-module-fixtures

    # Remove the temporary repository directory, which should contain all the
    # metadata, targets, and key files generated of all the test cases.
    shutil.rmtree(cls.temporary_directory)

    # Kill the SimpleHTTPServer process.
    if cls.server_process.returncode is None:
      logger.info('Server process ' + str(cls.server_process.pid) + ' terminated.')
      cls.server_process.kill()
      cls.server_process.wait()

If the server has files open in the temp directory, rmtree cannot succeed. Server should be killed (and waited on) first.

[MVrachev]

I am thinking that this problem could appear in multiple places.

With a quick search of the line shutil.rmtree(cls.temporary_directory) I found a couple of files which contain this line and are using a server subprocess:

• tests/test_arbitrary_package_attack.py
• tests/test_endless_data_attack.py
• tests/test_extraneous_dependencies_attack.py
• tests/test_indefinite_freeze_attack.py
• tests/test_key_revocation_integration.py
• tests/test_mix_and_match_attack.py
• tests/test_replay_attack.py
• tests/test_updater_root_rotation_integration.py
• tests/test_updater.py

[jku]

rmtree is only an issue because the server is still running in this case: so switching the order of rmtree and server kill here (and elsewhere if there's other places where the order is wrong) should help

[MVrachev]

rmtree is only an issue because the server is still running in this case: so switching the order of rmtree and server kill here (and elsewhere if there's other places where the order is wrong) should help

Yes, the cleanup in the files I have mentioned above are using the same order: remove the folder before stop the subprocess.

[MVrachev]

@joshuagl I forgot to use the Fixes keyword in Log subproceses stdout and stderr in temp files #1104 pr to automatically close this one. You can close it.