Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable debug mode when debugger is detected #2120

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Nov 18, 2024

What does this implement/fix?

During #2112, it became an issue that gdb is only able to follow one process at a time. This can either be (a) the main process, causing any forks to run unsupervised, or (b) following the new fork, either (b.1) detaching from the main process or (b.2) freezing the main process while the fork is being traced. (a) is the default behavior.

All three possibilities are not really what we want. This PR adds a mechanism to detect if the debugger is being attached and - if so - stops forking temporarily until the debugger is again detached. Even when this has a possible performance impact on TCP connections, it allows gdb to follow the program closely and will help us to debug crashes only becoming visible in TCP forks.


Related issue or feature (if applicable): #2112

Pull request in docs with documentation (if applicable): N/A


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

…hed. This allows us to debug TCP workers as well as they won't fork

Signed-off-by: DL6ER <[email protected]>
…ssume not being debugged in this case

Signed-off-by: DL6ER <[email protected]>
src/shmem.c Dismissed Show resolved Hide resolved
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Even after attaching gdb it loggs some vforks. Are those real forks or something else?

(gdb) continue
Continuing.
[Detaching after vfork from child process 735265]
[Detaching after vfork from child process 735267]
[Detaching after vfork from child process 735269]
[Detaching after vfork from child process 735271]

@DL6ER
Copy link
Member Author

DL6ER commented Nov 18, 2024

vforks are special and are probably unrelated to "real" TCP forks. They may be the consequence of pihole-FTL calling arp or any other CLI process - did you observe actual forks (can be seen in the log as 1234/F678)?

Even with this PR, it is expected that other processes we are calling are considered forks - we don't want to debug, e.g., ip address show

@yubiuser
Copy link
Member

did you observe actual forks

Not within the last 10 hours.

@@ -1471,3 +1472,82 @@ void print_recycle_list_fullness(void)
log_info(" Domains: %u/%u (%.2f%%)", recycler->domain.count, RECYCLE_ARRAY_LEN, (double)recycler->domain.count / RECYCLE_ARRAY_LEN * 100.0);
log_info(" DNS Cache: %u/%u (%.2f%%)", recycler->dns_cache.count, RECYCLE_ARRAY_LEN, (double)recycler->dns_cache.count / RECYCLE_ARRAY_LEN * 100.0);
}

/**
* @brief Dumps the string table to a temporary file.
Copy link
Member

Choose a reason for hiding this comment

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

What kind of strings are dumped here? Do we need to make sure that no privacy related information are dumped?

Copy link
Member Author

Choose a reason for hiding this comment

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

All strings FTL knows about. Mainly domains, host names, and IP addresses. But it can also be group strings (like 0,1,2,3) or local interface names or MAC addresses (if clients are configured by these).

Debugging is not really concerned about privacy levels, e.g., debug.queries also always prints the full strings as FTL needs them internally to handle to queries correctly. Only API responses and database records are affected by privacy levels.

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

Successfully merging this pull request may close these issues.

2 participants