Repository: bitcoin/bitcoin
File: test/functional/test_framework/test_node.py (Python)

Code before:

        del self.p2ps[:]

        assert (not expected_stderr) or wait_until_stopped  # Must wait to check stderr
        if wait_until_stopped:
            self.wait_until_stopped(expected_stderr=expected_stderr)

    def is_node_stopped(self, *, expected_stderr="", expected_ret_code=0):
        """Checks whether the node has stopped.

        Returns True if the node has stopped. False otherwise.
        This method is responsible for freeing resources (self.process)."""
        if not self.running:
            return True
        return_code = self.process.poll()
        if return_code is None:
            return False

        # process has stopped. Assert that it didn't return an error code.
        assert return_code == expected_ret_code, self._node_msg(
            f"Node returned unexpected exit code ({return_code}) vs ({expected_ret_code}) when stopping")
        # Check that stderr is as expected
        self.stderr.seek(0)
        stderr = self.stderr.read().decode('utf-8').strip()
        if stderr != expected_stderr:
            raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))

        self.stdout.close()
        self.stderr.close()

        self.running = False
        self.process = None
        self.rpc_connected = False
        self.rpc = None
        self.log.debug("Node stopped")
        return True

    def wait_until_stopped(self, *, timeout=BITCOIND_PROC_WAIT_TIMEOUT, expect_error=False, **kwargs):
        if "expected_ret_code" not in kwargs:
            kwargs["expected_ret_code"] = 1 if expect_error else 0  # Whether node shutdown return EXIT_FAILURE or EXIT_SUCCESS
        self.wait_until(lambda: self.is_node_stopped(**kwargs), timeout=timeout)

    def kill_process(self):
        self.process.kill()
        self.wait_until_stopped(expected_ret_code=1 if platform.system() == "Windows" else -9)
        assert self.is_node_stopped()

    def replace_in_config(self, replacements):
        """
        Perform replacements in the configuration file.
        The substitutions are passed as a list of search-replace-tuples, e.g.

Review comment (bug):
In commit "test: Kill process if stop-RPC is not available" (4ab7879ec6b71804cc3a51bcf58fa6318fa103b0)

I think this is probably the simplest way to write this code and we should stick with it for now, but just want to point out there is a minor race condition here because the process could end between the time that poll and kill are called and then the kill call could throw an exception.

Another separate idea is that it might be safer to write `self.expected_ret_code = self.process.wait()` instead of hardcoding 1/-9 return codes. Again though would be inclined to stick with the working code unless there is a more of a reason to change.

Code after:
        if not self.running:
            return
        assert self.rpc_connected, self._node_msg(
            "Should only call stop_node() on a running node after wait_for_rpc_connection() succeeded. "
            f"Did you forget to call the latter after start()? Not connected to process: {self.process.pid}")
        self.log.debug("Stopping node")
        # Do not use wait argument when testing older nodes, e.g. in wallet_backwards_compatibility.py
        if self.version_is_at_least(180000):
            self.stop(wait=wait)
        else:
            self.stop()

        # If there are any running perf processes, stop them.
        for profile_name in tuple(self.perf_subprocesses.keys()):
            self._stop_perf(profile_name)

        del self.p2ps[:]

        assert (not expected_stderr) or wait_until_stopped  # Must wait to check stderr
        if wait_until_stopped:
            self.wait_until_stopped(expected_stderr=expected_stderr)

    def is_node_stopped(self, *, expected_stderr="", expected_ret_code=0):
        """Checks whether the node has stopped.

        Returns True if the node has stopped. False otherwise.
        This method is responsible for freeing resources (self.process)."""
        if not self.running:
            return True
        return_code = self.process.poll()
        if return_code is None:
            return False

        # process has stopped. Assert that it didn't return an error code.
        assert return_code == expected_ret_code, self._node_msg(
            f"Node returned unexpected exit code ({return_code}) vs ({expected_ret_code}) when stopping")
        # Check that stderr is as expected
        self.stderr.seek(0)
        stderr = self.stderr.read().decode('utf-8').strip()
        if stderr != expected_stderr:
            raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))

        self.stdout.close()
        self.stderr.close()

        self.running = False
        self.process = None
        self.rpc_connected = False
        self.rpc = None
        self.log.debug("Node stopped")
        return True