diff --git a/recipes/recipe_modules/bot_update/resources/bot_update.py b/recipes/recipe_modules/bot_update/resources/bot_update.py index 08d92ed08..8ccb2112c 100755 --- a/recipes/recipe_modules/bot_update/resources/bot_update.py +++ b/recipes/recipe_modules/bot_update/resources/bot_update.py @@ -134,6 +134,7 @@ class RepeatingTimer(threading.Thread): self.kwargs = kwargs if kwargs is not None else {} self.cond = threading.Condition() self.is_shutdown = False + self.is_reset = False def reset(self): """Resets timer interval.""" @@ -191,6 +192,8 @@ def call(*args, **kwargs): # pragma: no cover env = os.environ.copy() env.update(new_env) kwargs['env'] = env + stale_process_duration = env.get('STALE_PROCESS_DURATION', + STALE_PROCESS_DURATION) if new_env: print('===Injecting Environment Variables===') @@ -200,40 +203,46 @@ def call(*args, **kwargs): # pragma: no cover print('In directory: %s' % cwd) start_time = time.time() proc = subprocess.Popen(args, **kwargs) - if stdin_data: - proc.stdin.write(stdin_data) - proc.stdin.close() - stale_process_duration = env.get('STALE_PROCESS_DURATION', - STALE_PROCESS_DURATION) observers = [ RepeatingTimer(300, _print_pstree), RepeatingTimer(int(stale_process_duration), _terminate_process, [proc])] + for observer in observers: observer.start() - # This is here because passing 'sys.stdout' into stdout for proc will - # produce out of order output. - hanging_cr = False - while True: - for observer in observers: - observer.reset() - buf = proc.stdout.read(BUF_SIZE) - if not buf: - break - if hanging_cr: - buf = '\r' + buf - hanging_cr = buf.endswith('\r') + + try: + # If there's an exception in this block, we need to stop all observers. + # Otherwise, observers will be spinning and main process won't exit while + # the main thread will be doing nothing. + if stdin_data: + proc.stdin.write(stdin_data) + proc.stdin.close() + # This is here because passing 'sys.stdout' into stdout for proc will + # produce out of order output. + hanging_cr = False + while True: + for observer in observers: + observer.reset() + buf = proc.stdout.read(BUF_SIZE) + if not buf: + break + if hanging_cr: + buf = '\r' + buf + hanging_cr = buf.endswith(b'\r') + if hanging_cr: + buf = buf[:-1] + buf = buf.replace(b'\r\n', b'\n').replace(b'\r', b'\n') + _stdout_write(buf) + out.write(buf) if hanging_cr: - buf = buf[:-1] - buf = buf.replace('\r\n', '\n').replace('\r', '\n') - _stdout_write(buf) - out.write(buf) - if hanging_cr: - _stdout_write('\n') - out.write('\n') - for observer in observers: - observer.shutdown() + _stdout_write(b'\n') + out.write(b'\n') + + code = proc.wait() + finally: + for observer in observers: + observer.shutdown() - code = proc.wait() elapsed_time = ((time.time() - start_time) / 60.0) outval = out.getvalue().decode('utf-8') if code: diff --git a/tests/bot_update_coverage_test.py b/tests/bot_update_coverage_test.py index 1c86d492d..cbd9fb94f 100755 --- a/tests/bot_update_coverage_test.py +++ b/tests/bot_update_coverage_test.py @@ -251,7 +251,7 @@ class BotUpdateUnittests(unittest.TestCase): bot_update.ensure_checkout(**self.params) args = self.gclient.records[0] patch_refs = set( - args[i+1] for i in xrange(len(args)) + args[i+1] for i in range(len(args)) if args[i] == '--patch-ref' and i+1 < len(args)) self.assertIn(self.params['patch_refs'][0], patch_refs) self.assertIn(self.params['patch_refs'][1], patch_refs) @@ -286,6 +286,10 @@ class BotUpdateUnittests(unittest.TestCase): actual_results = bot_update.parse_revisions(revisions, 'root') self.assertEqual(expected_results, actual_results) +class CallUnitTest(unittest.TestCase): + def testCall(self): + ret = bot_update.call(sys.executable, '-c', 'print(1)') + self.assertEqual(u'1\n', ret) if __name__ == '__main__': unittest.main()