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

Code before:
            # this destructor is called.
            print(self._node_msg("Cleaning up leftover process"))
            self.process.kill()

    def __getattr__(self, name):
        """Dispatches any unrecognised messages to the RPC connection or a CLI instance."""
        if self.use_cli:
            return getattr(RPCOverloadWrapper(self.cli, True, self.descriptors), name)
        else:
            assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
            return getattr(RPCOverloadWrapper(self.rpc, descriptors=self.descriptors), name)

    def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, env=None, **kwargs):
        """Start the node."""
        if extra_args is None:
            extra_args = self.extra_args

        # If listening and no -bind is given, then bitcoind would bind P2P ports on
        # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is
        # a unique port chosen by the test framework and configured as port=P in
        # bitcoin.conf. Don't bind on 127.0.0.1:18445 because it is not needed and
        # to avoid collisions.
        will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args)
        has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args)
        if will_listen and not has_explicit_bind:
            extra_args.append("-bind=0.0.0.0")

        self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args)

        # Add a new stdout and stderr file each time bitcoind is started
        if stderr is None:
            stderr = tempfile.NamedTemporaryFile(dir=self.stderr_dir, delete=False)
        if stdout is None:
            stdout = tempfile.NamedTemporaryFile(dir=self.stdout_dir, delete=False)
        self.stderr = stderr
        self.stdout = stdout

        if cwd is None:
            cwd = self.cwd

        # Delete any existing cookie file -- if such a file exists (eg due to
        # unclean shutdown), it will get overwritten anyway by bitcoind, and
        # potentially interfere with our attempt to authenticate
        delete_cookie_file(self.datadir_path, self.chain)

        # add environment variable LIBC_FATAL_STDERR_=1 so that libc errors are written to stderr and not the terminal
        subp_env = dict(os.environ, LIBC_FATAL_STDERR_="1")
        if env is not None:
            subp_env.update(env)

        self.process = subprocess.Popen(self.args + extra_args, env=subp_env, stdout=stdout, stderr=stderr, cwd=cwd, **kwargs)

Review comment (suggestion):
Feels confusing to have the test framework diverge in behavior this much from default `bitcoind` behavior.

Would be less surprising if the test framework avoided Tor port collisions through spreading out the ports among the nodes as is done for (P2P)Port and RPCPort. A suggested alternative to your current workaround is in 6b785873557696cc611d58fdcac5a3d54622082c.

Code after:
            # Avoid using logger, as that may have already been shutdown when
            # this destructor is called.
            print(self._node_msg("Cleaning up leftover process"))
            self.process.kill()

    def __getattr__(self, name):
        """Dispatches any unrecognised messages to the RPC connection or a CLI instance."""
        if self.use_cli:
            return getattr(RPCOverloadWrapper(self.cli, True, self.descriptors), name)
        else:
            assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
            return getattr(RPCOverloadWrapper(self.rpc, descriptors=self.descriptors), name)

    def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, env=None, **kwargs):
        """Start the node."""
        if extra_args is None:
            extra_args = self.extra_args

        # If listening and no -bind is given, then bitcoind would bind P2P ports on
        # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is
        # a unique port chosen by the test framework and configured as port=P in
        # bitcoin.conf. To avoid collisions on 127.0.0.1:18445, change it to
        # 127.0.0.1:tor_port().
        will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args)
        has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args)
        if will_listen and not has_explicit_bind:
            extra_args.append(f"-bind=0.0.0.0:{p2p_port(self.index)}")
            extra_args.append(f"-bind=127.0.0.1:{tor_port(self.index)}=onion")

        self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args)

        # Add a new stdout and stderr file each time bitcoind is started
        if stderr is None:
            stderr = tempfile.NamedTemporaryFile(dir=self.stderr_dir, delete=False)
        if stdout is None:
            stdout = tempfile.NamedTemporaryFile(dir=self.stdout_dir, delete=False)
        self.stderr = stderr
        self.stdout = stdout

        if cwd is None:
            cwd = self.cwd

        # Delete any existing cookie file -- if such a file exists (eg due to
        # unclean shutdown), it will get overwritten anyway by bitcoind, and
        # potentially interfere with our attempt to authenticate
        delete_cookie_file(self.datadir_path, self.chain)

        # add environment variable LIBC_FATAL_STDERR_=1 so that libc errors are written to stderr and not the terminal
        subp_env = dict(os.environ, LIBC_FATAL_STDERR_="1")
        if env is not None:
            subp_env.update(env)