Repository navigation
Bug: Corrupt or contended event subscription state could lead to termination of intact subscriber on Unsubscribe() #722
Copy link
Copy link
Open
Description
Description
In EventSubscriptionControl::Unsubscribe() any failure is handled as a fatal error which calls std::terminate(). If an application uses more than one service or other stateful functionality this could lead to the other state not being cleaned up any more and therefore in the worst case into cascading errors.
Error Occurrence Rate
No response
How to reproduce
No response
Supporting Information
communication/score/mw/com/impl/bindings/lola/event_subscription_control.cpp
Lines 129 to 160 in dfdeb8e
| for (auto current_retry = 0U; current_retry < max_retries; ++current_retry) | |
| { | |
| auto current_state = current_subscription_state_.load(); | |
| auto current_subscribers = GetSubscribersFromState(current_state); | |
| if (current_subscribers == 0U) | |
| { | |
| mw::log::LogFatal("lola") | |
| << "EventSubscriptionControl<>::Unsubscribe() Current subscriber count is already 0!"; | |
| std::terminate(); | |
| } | |
| SlotNumberType current_subscribed_slots = GetSubscribedSamplesFromState(current_state); | |
| if (current_subscribed_slots < slot_count) | |
| { | |
| mw::log::LogFatal("lola") | |
| << "EventSubscriptionControl<>::Unsubscribe() rejected as currently subscribed slots " | |
| "are smaller than slot_count."; | |
| std::terminate(); | |
| } | |
| std::uint32_t new_state = CreateState(static_cast<SubscriberCountType>(current_subscribers - 1U), | |
| static_cast<SlotNumberType>(current_subscribed_slots - slot_count)); | |
| auto success = AtomicIndirectorType<std::uint32_t>::compare_exchange_weak( | |
| current_subscription_state_, current_state, new_state, std::memory_order_acq_rel); | |
| if (success) | |
| { | |
| return; | |
| } | |
| } | |
| mw::log::LogFatal("lola") | |
| << "EventSubscriptionControl<>::Unsubscribe() retry limit exceeded, couldn't unsubscribe!"; | |
| std::terminate(); |
Classification
major
Affected Version
0.5
Expected Closure Version
No response
Category
- Safety Related
- Security Related
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
- StatusShow more project fieldsBacklog
Activity
limdor commentedon Jul 22, 2026
Hi, thanks for raising the issue. We are aware that we have several places in code where we call
std::terminate. We know this can be problematic in some use cases but the solution is not trivial. For this specific method, we could propagate the error but then the caller has the same decision problem. If the caller is a destructor insidemw::com, then we cannot propagate the issue further without exceptions.This would require a major refactoring.
It is fine to keep this issue opened but I just wanted to point out that it will most probably not be fixed soon.
If you have some concrete proposal to tackle this specific use case please let us know and we can consider.
crimson11 commentedon Jul 27, 2026
Something to add on top of what @limdor mentioned:
As you know: Currently the
Subscribe()andUnsubscribe()is done completely on behalf of the consumer/proxy. It updates the shared "subscription state" in shared-memory. This has already a bug/known issue: The maxSubscribers/maxSlots is currently NOT correctly supervised across QM and ASIL-B consumers! I.e. if the provider configuredmaxSubscribers = 3this semantically means "maxSubscribers = 3 across QM and ASIL-B subscribers` ... but this is not correctly checked! Each subscriber (ASIL-B or QM) can technically only check QM or ASIL-B part, but not overarching!So we are already thinking about changing Subscribe/Unsubscribe being refactored completely to use message-passing and let the logic de done by the skeleton ... this would change everything and the ticket might then be obsolete.