[Kilted] Don't pause recorder after stop

[brow1633]

Description

Removes pause from stop, as in #2313 from rolling.

Fixes # 2409

Is this user-facing behavior change?

Yes, but should fix a regression.

Did you use Generative AI?

Claude Opus 4.6 assisted with triage.

Additional Information

Can be back ported to Jazzy as well.

[MichaelOrlov]

This pause was introduced on purpose to mitigate undefined behaviour.

The problem is that the subscription's callbacks are propagated to the executor and may still be in the executor's queue after finishing the stop() operation.
Using a pause was to ensure that even if callbacks from deleted subscriptions would be called, they wouldn't propagate and call the deleted writer.
Of course pause is not completely fixing the undefined behaviour, but removing it will make it worse and more often will cause unexpected crashes.

The proper fix is to use a new API disable_callbacks as in the #2313, but unfortunately, we can't backport it.

[brow1633]

This pause was introduced on purpose to mitigate undefined behaviour.

The problem is that the subscription's callbacks are propagated to the executor and may still be in the executor's queue after finishing the stop() operation. Using a pause was to ensure that even if callbacks from deleted subscriptions would be called, they wouldn't propagate and call the deleted writer. Of course pause is not completely fixing the undefined behaviour, but removing it will make it worse and more often will cause unexpected crashes.

The proper fix is to use a new API disable_callbacks as in the #2313, but unfortunately, we can't backport it.

Ah, understood. In that case, can we reset the paused_ state when calling record()? Having pause here (and a lack of a reset in record()) changed behavior. More info: #2409

[MichaelOrlov]

As regards

In that case, can we reset the paused_ state when calling record()?

Unfortunately, not because in this case we will break another feature start-paused. We do have a dedicated CLI option --start-paused initialized here

rosbag2/rosbag2_transport/src/rosbag2_transport/recorder.cpp

Line 204 in 9687a11

paused_(record_options.start_paused),

The workaround would be to call resume() API or service call explicitly after the stop() and before the next record() API calls.

Update: or even better to call resume() API after the second record() call to avoid undefined behavior.

[brow1633]

The current behavior seems to break the most common/obvious use case, but yes we have implemented the workaround. With start paused set to false, I would never expect to need to resume when calling record again or after calling stop. As an anecdote, due this change, we lost several terabytes of data in a single day.

…

On Wed, Apr 22, 2026 at 9:54 PM Michael Orlov ***@***.***> wrote: *MichaelOrlov* left a comment (ros2/rosbag2#2410) <#2410?email_source=notifications&email_token=AXLCDG6XDVM6L6ZNI2CGI4L4XGOZXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMZQGE4DCMRZHA42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2LK4DSL5RW63LNMVXHIX3POBSW4X3DNRUWG2Y#issuecomment-4301812989> As regards In that case, can we reset the paused_ state when calling record()? Unfortunately, not because in this case we will break another feature start-paused. We do have a dedicated CLI option --start-paused initialized here https://github.com/ros2/rosbag2/blob/9687a1111b22ecd0866d807072a79bad7275dbad/rosbag2_transport/src/rosbag2_transport/recorder.cpp#L204 The workaround would be to call resume() API or service call explicitly after the stop() and before the next record() API calls. — Reply to this email directly, view it on GitHub <#2410?email_source=notifications&email_token=AXLCDG6XDVM6L6ZNI2CGI4L4XGOZXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMZQGE4DCMRZHA42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2LK4DSL5RW63LNMVXHIX3POBSW4X3DNRUWG2Y#issuecomment-4301812989>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXLCDG6WATPKSGOC2HWYBED4XGOZXAVCNFSM6AAAAACYBZGWMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGMBRHAYTEOJYHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[MichaelOrlov]

@brow1633

As an anecdote, due this change, we lost several terabytes of data in a single day.

Sorry about this. To make a proper fix, I had to make significant changes on a RCLCPP layer. It was a fundamental problem in the ROS 2 executor design.

[MichaelOrlov]

@brow1633 I created a follow-up PR #2412 to outline this behavior as a known limitation in the README file.

[MichaelOrlov]

• Closing as wouldn't be merged. Since pause on stop was introduced as a workaround to mitigate undefined behavior when callbacks from subscriptions are called after the subscription's destruction.
• The current deviated behavior was documented in the README.md file and headers in the follow-up [kilted] Add known limitations section to README regarding recorder restart behavior #2412 PR.