Find the running core port and use it in GUI #7162

xoriole · 2022-11-14T12:52:36Z

This PR adds a PortChecker class that finds the closest port opened by a process identified by given pid and the base port.
A callback can also be set which is triggered when the port is detected.

    port_checker = PortChecker(pid, base_port, callback)
    port_checker.start_checking()

    # The detected port can be retrieved as:
    port_checker.detected_port

The port checker is used by the CoreManager to find the core API port.

When the port is detected, request manager and event manager are updated so they can operate with the correct port.

Further, it also adds two environment variables TRIBLER_API_PORT_INITIAL and TRIBLER_API_PORT_DETECTED with values initial and detected api port values. This is so that these variables will be made available in sentry report if there is a crash.

Fixes #7137

kozlovsky · 2022-11-28T08:18:45Z

src/tribler/gui/core_manager.py

@@ -118,6 +125,7 @@ def start_tribler_core(self):
 connect(self.core_process.finished, self.on_core_finished)
 self._logger.info(f'Start Tribler core process {sys.executable} with arguments: {core_args}')
 self.core_process.start(sys.executable, core_args)
+ self.port_checker.setup_with_pid(self.core_process.processId())


Unfortunately, this logic currently does not work on Windows, as self.core_process.processId() returns an incorrect PID value, and then the port checker cannot find open ports for a core process.

According to this, the PID value can be incorrect if asked too quickly. You need to be sure that the process is actually started, so this logic should be moved to on_core_started. I tried it, but it didn't help to fix the problem.

I suspect an incorrect PID value can result from a bug in the QProcess.processId() implementation for Windows.

There is also a low-level method, self.core_process.pid(); I also tried it. The result is OS-dependent; on Unix, it is a normal PID value, whereas, on Windows, it is an instance of a sip.voidptr object. This voidptr should point to a _PROCESS_INFORMATION structure. In principle, extracting the PID value from this structure should be possible using ctypes. I tried the following:

ptr_as_int = self.core_process.pid().__int__() import ctypes class _PROCESS_INFORMATION(ctypes.Structure): _fields_ = [("hProcess", ctypes.c_int), ("hThread", ctypes.c_int), ("dwProcessID", ctypes.c_int), ("dwThreadID", ctypes.c_int)] process_information = ctypes.cast(ctypes.c_void_p(ptr_as_int), ctypes.POINTER(_PROCESS_INFORMATION)) pid = process_information.dwProcessID

But the resulting PID value is still incorrect. Maybe I did something wrong with ctypes here.

Example values from a debug session:

Actual GUI process PID: 32704

Actual Core process PID: 34640

core_process.processId() value: 31468

core_process.pid() value: <sip.voidptr object at 0x0000014984C63D20>

core_process.pid().__int__() value: 1415218422960

process_information.dwProcessID value: 2372

process_information.hProcess value: 2376

In conclusion, at this moment, I don't see any reliable way to get a correct PID value of the QProcess instance on Windows. As a simple alternative, we can do one of the following:

Extract the PID value from the triblerd.lock file using ProcessChecker. I modified the code this way, and it works, so it looks like the easiest way to fix the issue.

Pass PID as a first stdout output of the Core process, and parse it in the GUI process.

As tested the pid is working on all machines.

As it turns out, the reason it does not work correctly on my PC is venv. Venv creates a separate python.exe wrapper that calls the actual Python inside. So, the PID of the Core process that QProcess returns is the PID of the venv wrapper and not of the actual Core process. As most Tribler users use the compiled version of Tribler, they should not experience this problem. Nevertheless, we should be aware of this venv behavior.

On my machine, the PR is working, but not in the way it may be expected. As the actual PID of the Core process is different from what QProcess returns, the PortChecker cannot find the correct port, so it never calls the EventRequestManager.update_port method. Instead, the timeout of EventRequestManager.connect_timer forces the request with the default port value, which turns out to be the correct one.

src/tribler/gui/port_checker.py

src/tribler/gui/event_request_manager.py

src/tribler/gui/port_checker.py

kozlovsky · 2022-12-16T08:13:33Z

src/tribler/gui/port_checker.py

+ def check_port(self):
+ self._logger.info(f"Checking ports; Base Port: {self.base_port}")
+ self.detect_port_from_process()
+ if self.detected_port and self._callback:


The checker timer will be restarted if the port is detected, but a callback is not set. So, if a callback is not specified, the PortChecker will continue work indefinitely, which looks like an incorrect behavior.

kozlovsky · 2022-12-16T08:21:44Z

src/tribler/gui/core_manager.py

@@ -122,8 +125,20 @@ def start_tribler_core(self):
 def on_core_started(self):
 self.core_started = True
 self.core_running = True
+ self.start_port_checker()


I think the CoreManager.stop() method should stop the port checker as well

kozlovsky · 2022-12-16T08:24:47Z

src/tribler/gui/core_manager.py

 self.events_manager.connect(reschedule_on_err=True) # retry until REST API is ready

+ def start_port_checker(self):


It may be a bit safer to check if self.port_checker is None at the beginning of the method for a case when the method is called twice after some future refactoring.

kozlovsky · 2022-12-16T09:26:59Z

src/tribler/gui/core_manager.py

 self.events_manager.connect(reschedule_on_err=True) # retry until REST API is ready

+ def start_port_checker(self):
+ core_process_id = self.core_process.processId()


Can you add a comment for future developers that when using venv on Windows, the PID value returned from the QProcess.processId is incorrect, as it is the PID of the venv wrapper process and not the PID of the Core process itself? Something like:

Suggested change

core_process_id = self.core_process.processId()

# When using venv on Windows, the Core process PID value returned by QProcess is incorrect,

# as it is the PID of the venv wrapper process and not the PID of the actual Core process.

# In that case, PortChecker finds nothing, and the default port is used.

core_process_id = self.core_process.processId()

kozlovsky · 2022-12-16T09:27:22Z

src/tribler/gui/core_manager.py

@@ -118,6 +125,7 @@ def start_tribler_core(self):
 connect(self.core_process.finished, self.on_core_finished)
 self._logger.info(f'Start Tribler core process {sys.executable} with arguments: {core_args}')
 self.core_process.start(sys.executable, core_args)
+ self.port_checker.setup_with_pid(self.core_process.processId())


As it turns out, the reason it does not work correctly on my PC is venv. Venv creates a separate python.exe wrapper that calls the actual Python inside. So, the PID of the Core process that QProcess returns is the PID of the venv wrapper and not of the actual Core process. As most Tribler users use the compiled version of Tribler, they should not experience this problem. Nevertheless, we should be aware of this venv behavior.

On my machine, the PR is working, but not in the way it may be expected. As the actual PID of the Core process is different from what QProcess returns, the PortChecker cannot find the correct port, so it never calls the EventRequestManager.update_port method. Instead, the timeout of EventRequestManager.connect_timer forces the request with the default port value, which turns out to be the correct one.

kozlovsky · 2023-01-11T10:26:46Z

Based on the ideas and code from this PR, PR #7251 was implemented with a different way of passing information about the opened port

xoriole force-pushed the fix/api-core-port branch 4 times, most recently from 390e3e6 to d3b1091 Compare November 17, 2022 13:38

drew2a force-pushed the main branch 5 times, most recently from d534eca to 4ea5f08 Compare November 21, 2022 13:47

xoriole requested a review from kozlovsky November 21, 2022 14:06

xoriole force-pushed the fix/api-core-port branch 2 times, most recently from 1345bca to 69ac6d8 Compare November 22, 2022 12:03

xoriole marked this pull request as ready for review November 22, 2022 12:12

xoriole requested a review from a team as a code owner November 22, 2022 12:12

kozlovsky requested changes Nov 28, 2022

View reviewed changes

xoriole marked this pull request as draft November 29, 2022 09:36

xoriole force-pushed the fix/api-core-port branch 2 times, most recently from 12bcd30 to d80d464 Compare November 29, 2022 12:30

xoriole added 2 commits December 7, 2022 15:11

Add port checker to detect core REST API port

4e58389

Add pytest-qt test package

af0b518

xoriole force-pushed the fix/api-core-port branch from 74d4873 to af0b518 Compare December 7, 2022 14:12

xoriole added 4 commits December 13, 2022 10:22

Update port checker tests

43ecdb1

Fix pylint issues on port checker tests

36a4d99

Add logger

b84d583

Add Tribler API ports (initial and detected) to environment variable

fea02ae

xoriole marked this pull request as ready for review December 13, 2022 11:03

xoriole requested a review from kozlovsky December 13, 2022 11:04

kozlovsky requested changes Dec 16, 2022

View reviewed changes

kozlovsky mentioned this pull request Jan 10, 2023

Pass the number of the opened REST API port from Core to GUI #7251

Merged

kozlovsky closed this Jan 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Find the running core port and use it in GUI #7162

Find the running core port and use it in GUI #7162

xoriole commented Nov 14, 2022 •

edited

Loading

kozlovsky Nov 28, 2022 •

edited

Loading

xoriole Dec 13, 2022

kozlovsky Dec 16, 2022

kozlovsky Dec 16, 2022

kozlovsky Dec 16, 2022

kozlovsky Dec 16, 2022

kozlovsky Dec 16, 2022

kozlovsky Dec 16, 2022

kozlovsky commented Jan 11, 2023

		self.events_manager.connect(reschedule_on_err=True) # retry until REST API is ready

		def start_port_checker(self):

- core_process_id = self.core_process.processId()
+ # When using venv on Windows, the Core process PID value returned by QProcess is incorrect,
+ # as it is the PID of the venv wrapper process and not the PID of the actual Core process.
+ # In that case, PortChecker finds nothing, and the default port is used.
+ core_process_id = self.core_process.processId()

Find the running core port and use it in GUI #7162

Find the running core port and use it in GUI #7162

Conversation

xoriole commented Nov 14, 2022 • edited Loading

kozlovsky Nov 28, 2022 • edited Loading

Choose a reason for hiding this comment

xoriole Dec 13, 2022

Choose a reason for hiding this comment

kozlovsky Dec 16, 2022

Choose a reason for hiding this comment

kozlovsky Dec 16, 2022

Choose a reason for hiding this comment

kozlovsky Dec 16, 2022

Choose a reason for hiding this comment

kozlovsky Dec 16, 2022

Choose a reason for hiding this comment

kozlovsky Dec 16, 2022

Choose a reason for hiding this comment

kozlovsky Dec 16, 2022

Choose a reason for hiding this comment

kozlovsky commented Jan 11, 2023

xoriole commented Nov 14, 2022 •

edited

Loading

kozlovsky Nov 28, 2022 •

edited

Loading