Skip to content

Fix TorControlSocket shutdown hang by adding timeout#256

Open
MHT417 wants to merge 2 commits intoRetroShare:masterfrom
MHT417:master
Open

Fix TorControlSocket shutdown hang by adding timeout#256
MHT417 wants to merge 2 commits intoRetroShare:masterfrom
MHT417:master

Conversation

@MHT417
Copy link

@MHT417 MHT417 commented Jan 30, 2026

TorControlSocket::process() has an infinite loop that doesn't check the
thread stop flag, this causes the thread to loop forever

Added timeout check to allow clean thread termination.

Copy link
Contributor

@csoler csoler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question pending.


if(line.empty()) // This happens when the incoming buffer isn't empty yet doesn't have a full line already.
{
if (++empty_line_cnt > MAX_EMPTY_RETRIES || shouldStop()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens here is the network is somehow stuck (due to some lag)? The TorControlSocket process will stop. I'm not sure you want that, unless there is a built-in mechanism to get it to restart when needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gets stuck when quitting a tor node, or occasionally even without quitting. the thread will just wait forever unless a timeout exists. it was tested again on a tor node after applying a timeout and the issue never occurred, RS will run without getting stuck

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I mispelled. I wanted to say "what happens IF the network is somehow stuck".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants