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

Feature: Support reports on demand #12

Closed

Conversation

rjpdasilva
Copy link
Contributor

The tool currently only produces reports on a clean exit from the analyzed program, which is not always achievable (daemons, programs crashing, etc.). An "on demand" report requested programmatically from within the application itself or from the "outside world" via a signal, has proven quite useful in multiple debugging scenarios.

Commit 4acca12 allows requesting reports via a new API function named hu_report().
Commit 2de0126 allows requesting reports via SIGUSR1 Linux signals.
Commit 3d60e08 fixes log_summary() so that can be called multiple times while the program is running.

The tool currently only produces reports on a clean exit from the
analyzed program, which is not always achievable (daemons, programs
crashing, etc.). An "on demand" report requested from within the
application itself has proven useful in debugging scenarios, e.g. by
attaching a debugger to the analyzed program and then just call a
routine from there for producing a `heapusage` report.

This way, extend `heapusage` with a public API, currently only having a
`hu_report()` function for generating a `heapusage` report as done from
`log_summary()`. The new API files are `huapi.h` and `huapi.cpp`. The
header file first gets installed in the standard include dir so other SW
modules can use it. The source file is added to the library's link step.

The `log_summary()` call for generating the report from `hu_report()` is
wrapped by a new `log_summary_safe()` function, so that `heapusage`
analysis is temporarily disabled when actually calling `log_summary()`,
otherwise `log_summary()` itself, which does memory allocations, would
interfere with the analysis. This problem didn't exist before, because
`log_summary()` was only being called from the exit handler, which also
disables the analysis before calling `log_summary()`.

Also add an internal global `log_message()` function, useful for logging
generic messages into the `heapusage` log.

Update `README.md` with this new usage possibility.

Signed-off-by: Ricardo Silva <[email protected]>
The `log_summary()` function is causing problems when being called
multiple times in the same run, i.e. if called multiple process while
the program being analyzed is running.

The observed problems are:

 * The `size` and `count` fields for leaked blocks for the same
   callstack are accumulating between calls, messing with the
   statistics.

 * If there are previous calls to `log_summary()` from e.g. on demand
   report requests done by using `hu_report()`, the last call done when
   the program exits (from `hu_fini()`), causes crashes (`SIGABRT` and
   `SIGSEGV` have been observed).

This all happens because the `allocations_by_callstack` map is being
used as `static`. This doesn't really seem to be needed or that useful,
since the size of the map is really small (measured as 24 bytes), so no
real memory savings by declaring it as static. There's also no observed
impact on the `log_summary()` performance by removing the `static`
attribute.

This way, let `allocations_by_callstack` be allocated from the stack and
restarted on every `log_summary()` call, hence avoiding the mentioned
problems.

Signed-off-by: Ricardo Silva <[email protected]>
The tool currently only produces reports on a clean exit from the
analyzed program, which is not always achievable (daemons, programs
crashing, etc.), and also using `hu_report()` as an API, which might
also not be doable in all scenarios (e.g. no source code for the
analyzed program). An "on demand" report requested from "the outside
world" has proven very useful in some debugging scenarios.

Having a signal that could be sent to the analyzed program (e.g. sent
from a shell) and have the `heapusage` tool catch it and produce a
report in response is very helpful.

This way, install a signal handler for the `SIGUSR1` and `SIGUSR2` user
defined signals and, for `SIGUSR1` trigger a report using
`log_summary_safe()`, just like the `hu_report()` API does. Leave the
`SIGUSR2` available for other future requests.

Update `README.md` with this new usage possibility.

Signed-off-by: Ricardo Silva <[email protected]>
@d99kris
Copy link
Owner

d99kris commented May 25, 2024

Hi @rjpdasilva - thanks for contributing 👍
At a quick glance I think the proposed changes look good! I will need to review a bit more in detail and test a little before merging. I'm going on vacation so it may take a week or two before I get to it.

Btw - do not worry about the failing macOS check for the PR, I suspect it's some general issue caused by changes to Github CI runners, so it's something I will address separately from this PR.

@d99kris d99kris self-assigned this May 25, 2024
@d99kris d99kris changed the base branch from master to integrate-on-demand June 15, 2024 02:06
@d99kris d99kris changed the base branch from integrate-on-demand to master June 15, 2024 04:44
@d99kris
Copy link
Owner

d99kris commented Jun 15, 2024

Hi @rjpdasilva - I've raised a PR for updating your fork with latest heapusage master so it will be easier for me to test it out.

@rjpdasilva
Copy link
Contributor Author

Hi @d99kris,

Hi @rjpdasilva - I've raised a PR for updating your fork with latest heapusage master so it will be easier for me to test it out.

If you prefer I can raise a new PR rebased on your latest master, no problem.

@d99kris
Copy link
Owner

d99kris commented Jun 15, 2024

Sure that's fine too, either works for me.

@rjpdasilva
Copy link
Contributor Author

Hi @d99kris,

Sure that's fine too, either works for me.

Done in #14.

@rjpdasilva rjpdasilva closed this Jun 15, 2024
@rjpdasilva rjpdasilva deleted the feature/reports_on_demand branch June 23, 2024 12:03
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