Skip to content

Conversation

@sverker
Copy link
Contributor

@sverker sverker commented Nov 5, 2025

Disabling tracing of a BIF in one trace session did affect other sessions tracing the same BIF.

@sverker sverker self-assigned this Nov 5, 2025
@sverker sverker added team:VM Assigned to OTP team VM fix testing currently being tested, tag is used by OTP internal CI labels Nov 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

CT Test Results

    3 files    142 suites   50m 35s ⏱️
1 652 tests 1 595 ✅ 57 💤 0 ❌
2 375 runs  2 298 ✅ 77 💤 0 ❌

Results for commit 449a07d.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@sverker sverker requested a review from Copilot November 5, 2025 13:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the handling of the is_bif_traced flag in export entries to work correctly with multiple trace sessions (OTP-19840). Previously, the flag was set/cleared incorrectly when multiple sessions were tracing the same BIF, leading to premature clearing or incorrect state.

  • Moved is_bif_traced flag management from erts_set_trace_pattern to erts_set_export_trace and consolidate_bp_data
  • Updated function signature of erts_set_export_trace to accept Export* instead of ErtsCodeInfo* to access BIF metadata
  • Added comprehensive test case is_bif_traced/1 to verify correct behavior with multiple trace sessions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
erts/emulator/beam/erl_bif_trace.c Removed incorrect single-session handling of is_bif_traced flag from erts_set_trace_pattern and prepare_clear_all_trace_pattern
erts/emulator/beam/beam_bp.h Updated function signature for erts_set_export_trace to accept Export*
erts/emulator/beam/beam_bp.c Implemented proper multi-session handling by setting flag in erts_set_export_trace and clearing in consolidate_bp_data when last session is removed
erts/emulator/beam/beam_bif_load.c Changed to assert that is_bif_traced is cleared during code unloading instead of manually clearing it
erts/emulator/test/trace_session_SUITE.erl Added test case to verify correct is_bif_traced flag management across multiple sessions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sverker sverker removed the testing currently being tested, tag is used by OTP internal CI label Nov 5, 2025
Problem:

Export.is_bif_traced was prematurely cleared when the first GenericBp
breakpoint was unlinked which caused tracing of the BIF for remaining
sessions to be ignored.

Solution:

Set 'is_bif_traced' if we have at least one GenericBp.
Clear 'is_bif_traced' when the last GenericBp disappears.
@sverker sverker force-pushed the sverker/erts/trace-bif-session-fix/OTP-19840 branch from a26f3ac to 449a07d Compare November 17, 2025 16:43
@sverker sverker added the testing currently being tested, tag is used by OTP internal CI label Nov 17, 2025
@sverker sverker added this to the 28.2 milestone Nov 17, 2025
@sverker sverker requested a review from jhogberg November 18, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant