Release 3.7.5 part 1 - #1064
Merged
Merged
Conversation
Migrate service commands (OFFER_SERVICE / STOP_OFFER_SERVICE) to the struct-based IPC approach. This follow-up introduces service_command_data, a value-type struct carrying service, instance, major and minor version, alongside dedicated create_offer_service_cmd() and create_stop_offer_service_cmd() factory functions. NoteThe former offer_service_command, stop_offer_service_command, and service_command_base classes are marked [[deprecated]] but remain functional — they will be removed once all dependent code paths are migrated in follow-up PRs. while at it, Refactor the simple commands a little bit further, which help us create more generic free functions. For reference: 1b1d43d
Increase the TTL of the offer test big sd msg Previously the test was failing because, since there are a lot of services being offered, it would take quite a while to process an offer message. This delay would lead to some offers expiring due to the wait between consecutive offers for the same service would exceed the normal TTL of 3 seconds, leading to the client to report the service as unavailable, and the client subscribing to some of the services more than once. To fix this, increase the TTL of the offers to a bigger number so that even under stress the the test does not fail.
The test had 2 problems: The client had both on_message and on_availability holding the mutex, which, with the sleeps preset in request_release, it would delay the increase of availability_handler_calls. When the server sent more messages, these could be processed before the on_availability and this could make the check to availability_handler_calls way too early. The other issue was the timings with the offers, now with the graceful period, some were not being transmitted to external clients and so, not trigger the availability handler. To fix this, unlock the mutex before calling request_release on client, and increase the OFFER_CYCLE_INTERVAL value to 2 seconds to space out the offers. Also a mutex cleanup on service.
Fix UDP client endpoint double connect issue that could cause a socket to be bound twice. The original race was caused by two different routing paths being able to start the same UDP client endpoint, leading to a double connect. The first path started in routing_manager_impl::request_service(), where a requested remote service caused ep_mgr_impl_->find_or_create_remote_client() to create and start the client endpoint. The second path started in routing_manager_impl::add_routing_info() when a later SD OFFER was processed for an already-known unreliable endpoint. In this specific path, if the endpoint exists but is not established yet, rmi will pass the ep->is_closed() verification and call ep->start() directly from routing_manager_impl. The new fix defines a new start_if_closed(), where we can lock the necessary mutex inside the endpoint and prevent double start operations.
Improve error handling logs in message reception There were some places where the message reception error handling logs could be improved, such as returns without any error log, and not using some functions return value where they could be useful in pin pointing where an error occurred. As such, improve the error handling logging.
To synchronize pending_offers and provided_services, the mutex pending_offers_mutex_ was removed in favor of provider_mutex_. Extracted host_.get_connection_param from create_local_client_unlocked and added into find_or_create_local_client outside of mtx_ to avoid a lock-order-inversion.
Add more info to error logs so its easier to whitelist them (specially for ETS logs).
Fix flaky test_boardnet_with_fake_sockets test_someip_gate.lets_through_n_notifications fails sporadically because the first duplicate notification is sent when the multicast offer subscription is sent before the unicast offer subscription has been acknowledged. As a result, we expect notification 0x33 to be blocked because it is the third notification, but when a duplicate notification occurs, the third notification is 0x22. To stabilize the test, we now check the session ID of the notifications instead of the payload, ensuring that we always verify the third notification.
MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate single-field commands (ASSIGN_CLIENT_ACK / OFFERED_SERVICES_REQUEST / RESEND_PROVIDED_EVENTS) to the struct-based IPC approach. This follow-up introduces single_field_command_data<T>, a generic value-type struct carrying a command_header and a single typed payload field, alongside dedicated factory functions create_assign_client_ack_cmd(), create_offered_services_request_cmd(), and create_resend_provided_events_cmd()`. NoteThe former assign_client_ack_command, offered_services_request_command, and resend_provided_events_command classes are marked [[deprecated]] but remain functional — they will be removed once all dependent code paths are migrated in follow-up PRs. While at it, refactor serialize to use a single constexpr-if dispatch instead of per-type function overloads. For reference: 1b1d43d fb7b611
Consolidate memcheck suppressions. Memcheck reporting "possibly lost" TLS storage for unjoined/detached threads is a classic false positive. We already had suppressions for the dispatcher, stop, I/O and offer threads, but test cases are still randomly failing in similar situations. Replace the specific suppressions by a wildcard one that catches this corner case for any thread.
standardize test scripts Some of the test scripts did not use the format that the majority of the test scripts use to wait for the test executable to finish and then obtain its return code. As such, make the test scripts use the same format everywhere.
To avoid two copies on send local_services_table is renamed into local_offer_table, and a new local_services_table is introduced that stores requests, into a continues area of memory. This allows to only serialize the requests into the send_queue, without any intermediate copy. Note that this implies that the serialization vs. deserialization implies a certain asymmetry, as the deserialization requires to copy the bytes into some sort of temporary container to avoid padding problems, while the serialization works without a temporary container.
Optimize restart_routing_test. Debug builds inherits an expected performance hit that had impact on slow and time sensitives tests. This PR optimizes the restart routing tests by running all in a single address space, no process spawning is required for this test case, this bring roughly a 10s reduction on each test fixture (debug build with parallel execution). Moreover, a duplicated test is removed.
Remove the offer commands Now that offers are correctly serialized with the offer_serialization_mutex_, the offer commands are no longer needed
Remove routing_manager_base
Use host-provided Boost libraries for network tests. Instead of manually building a specific version of Boost as part of the container setup, just use the one that comes with the host container (currently 1.83). Saves about a minute of build time per test run, but more importantly that newer version has some bugs fixed that impede on the stability of test runs (e.g. the undefined behavior in process environment handling that occasionally triggers sanitizers). We have validation builds that compile-test for specific versions of Boost, including 1.75, which shall stay around; but for running the network test suite, it does not make a lot of sense to stick with the lowest supported one if a majority of target platforms provide a different one anyway.
Fix external_local_* and routing tests to work reliably inside and outside a network sandbox. Some test cases check for open TCP or UDP ports, either to wait for specific ports to appear before going on, or for doublechecking that specific ports are not open when they should not be. However, the checks only checked for LISTEN state which does not actually exist for UDP. The checks pass only by accident, because there usually is an open TCP listener (sshd). However, that does not exist in a network sandbox. Fix this by also checking for UDP bound state (0x07). However, outside of a network sandbox, there may be an arbitrary number of unrelated ports open, which may cause the checks to pass prematurely. Fix this by explicitly checking for the expected port numbers rather than just counting open ports. Because the code becomes more complex and is used in a number of places, introduce a new shared test_helpers.sh and move the fixed functions there. For local communication, expecting an open socket is bogus anyway, so just remove that check (which also passed previously due to sshd). Also fix the exit code check for one of the tests, which accidentally used the wrong variable (EXTERNAL_CLIENT_PID instead of CLIENT_PID).
Fix a timing issue in suspend_resume_test_service. Under high load (e.g. running under valgrind), the daemon might be too slow to process the SIGUSR1 and SIGUSR2 fast enough, leading to a pile-up and subsequent reordering of these signals. Avoid this by letting the service wait for the subscription being active at the beginning of a new iteration, ensuring that the previous resume event was fully processed.
Fix local_routing_test if multiple dispatcher threads are active. If multiple dispatcher threads are spawned, responses may arrive out of order. Relax the session ID check so it does not rely on response order.
Make debounce_filter_test slightly more jitter-tolerant. Increase the tolerance for average interval checks slightly, in order to be more robust towards jitter. Especially under load (valgrind) this test otherwise sometimes misses the expected values just by an ms or two.
Turn the stopping of the rmc into an async operation. This is the last step enabling to run a vsomeip application (including multiple start, stop cycles), with a single io thread. Including the flushing of endpoints. The happy path execution is: Disable the creation of any new endpoint Start the flushing process for all endpoints When all endpoints are flushed, Cleanup "remote state" Let the stopping continue on application_impl It is assumed for now that the starting and stopping process can not overlap. Note that already before this PR the state of the application is not well defined when stop and then start are invoked in short repetition. To enable the orchestration the async::hook and async::trigger have been introduced.
Fix potential race in test_connection_restoration. Reorder expectations to check servive availability before clioent response. This matches what the companion test case below has a comment on, detailing why that order is important. This issue was identified to be the reason for a spurious test failure.
Fix offer stop offer With the introduction of the graceful stop offer mechanism (f7cac2b), the offer window of the offer stop offer was too narrow, since an offer sent immediately after a stop offer will only be sent remotely after a timeout has passed. This coincided with the next offer stop offer cycle, and as such the client would never see both services as available since it would receive a stop offer shortly after the offers. To prevent this, increase the offer window. This allows the client to receive one cyclic offer after the stop offers (and before receiving stop offers again), and gives enough time for the client to see both services as available.
Fuse offer_serialization_mutex_ and on_state_change_mutex_ as mutex_. Originally, in rmi::offer_service, external routing readiness was defined under on_state_change_mutex_ before offer_serialization_mutex_ was locked. This left a small window where routing could become ready and init_pending_services() could clear pending_sd_offers_, while another offer still had an older "not ready" value. If that offer then added its service to pending_sd_offers_ after the clear, the service would stay pending until the next transition to RS_RESUMED (which in our tests won't happen and on real scenarios might take a long time to happen). As these mutexes are almost always used together, this PR fuses them to prevent races (such as the one above) that become possible when we try to prevent reverse lock. The new mutex was named mutex_ in an intent of having it as our first general mutex for routing_manager_impl.
Fix instability in initial_event_test. Ensure that the routing daemon is (still) alive before starting clients or services that would wait for it. Otherwise, test cases might hang forever until global timeout is reached, while producing an enormous amount of logs. In particular, this fixes the case where (e.g. on a system under load) the routing daemon terminates faster than a service killed before it, which would cause the test harness to go into an infite loop.
It seems that a response to an external client is being lost when being added to the endpoint's train, however it leaves not trail of what could be happening wrong. Add logs where possible issues might arise Simplify checks
It makes sense to check whether the incoming message client-id matches the connection client-id, and this is independent of whether security is on or not, or whether this is UDS or TCP While at it, fix variable shadowing
Struct based to aggregate trivial commands Migrate fixed-size aggregate commands (RELEASE_SERVICE, UNREGISTER_EVENT, UNSUBSCRIBE_ACK, REMOVE_SECURITY_POLICY, UPDATE_SECURITY_POLICY_RESPONSE, REMOVE_SECURITY_POLICY_RESPONSE) to the struct-based IPC approach. Compatibility tests assert byte-for-byte equivalence between the old class-based and the new struct-based wire format in both directions for each migrated command. Round-trip tests cover create_* → serialize → deserialize for the new structs. NoteThe former commands classes are marked [[deprecated]] but remain functional -- they will be removed once all dependent code paths are migrated in follow-up PRs. For reference: 1b1d43d fb7b611 da84c20 dd4f7a8
Include file descriptor on status logs.
Modernize service_instance_t from encapsulated class to C++20 aggregate with public members and defaulted operator<=>. Replaces private service_/instance_ fields + getters with public service/instance members; Uses now direct member access; Simplifies comparison operators in event_data_t and subscription_data_t using std::tie (now possible since the struct itself is three-way comparable).
Revert the C++20 features that don't compile under Android's Clang 14.0.7 (clang-r450784d) Android's toolchain uses clang-r450784d (= Clang 14.0.7), which doesn't fully support C++20. Reverted (unsupported on Clang 14): std::ranges, container .contains() member functions. Kept (supported): designated initializers, <=>, std::erase_if().
Fix header_factory_test with multi-dispatch. If requests are handled by multiple dispatcher threads, responses may arrive out of order. Relax test expectations to deal with this fact, removing this timing issue.
Fix race condition in client_id_test_service. The service used the same flag 'blocked_' to wait for different things: ST_REGISTERED and all services being available. This could lead to a deadlock if events did not happen in the expected order, namely if the offer() call in run() happens after notifications for both registration and service availability are already in. Fix this by introducing a separate flag for service. Also fix another data race by moving the lock in on_availability before modifying the map it is supposed to protect.
Record SOME/IP and SD messages on gate Updated someip_gate to record SOME/IP and SD messages when the gate is installed.
Summary This PR changes minor files related to the network-tests.
Fix a race in test_boardnet. Wait for unavailability of the service after taking down the interface, but before clearing the list and validating that no new events occur. Otherwise, it can happen (e.g. under Valgrind) that the side effects expected from taking down the interface are not fully processed yet, and falsely trigger subsequent assertions.
Meanwhile remove the (de)serializer usage in rmc. This implies that during a send operation we avoid three additional copies + two mutex locks for the serializer, and avoid three more copies during the reception + two more mutex locks for the deserializer. The avoidance of the send_command in the rmi/rms, has less severe impact: One less copy on reception, two less copies on sending towards local clients.
Turn last_session into a property of the test_client class. The network test big_payload_test_local_random was failing due to a data race around the last_session variable, which was declared as a static variable, and therefore shared across all threads running a big_payload_test_client::on_message instances. The last_session variable was turned into a property of big_payload_test_client and a mutex was added to protect its use.
Replace subscribe_ack_command and subscribe_nack_command
It is only used for security checks, and those checks by themselves are flawed - incoming messages must be always checked against the connection they come from
Remove several magic sleeps from test cases. A number of magic sleeps had been added to test cases, mostly in order to give them time to shut down without losing relevant messages. However, such sleeps cause issues when test cases run in parallel or otherwise under load, as the delay may not be sufficient then to ensure the test case passing. Replace some of the sleeps where proper synchronization is straightforward, in order to improve robustness under load. There are still more magic sleeps remaining that would require more elaborate handling.
Fix race condition in restart_routing_test. If the service is destroyed before registration is finished, the service runner would wait indefinitely for that registration that will never come. Turns out that the test case does not even need to care about service registration, so remove related code altogether.
Prevent double start or netlink_connector. A race condition was (rarely) triggered by a unit test (specifically, ipv4_netlink_fixture.start_stop) that issues the netlink_connector::start() method twice in a row. Closing the socket mid-request would sometimes lead to lost messages, and thus fail the test. There is no safe and straightforward way to restart the connector (as the already existing comments in both production and test code also stated...). Instead of trying to do so, simply ignore the second start request. In production, only one start() is ever called anyway; it is only this test case that is trying this. Adapt the comment there accordingly.
ucei will read UDP messages, one-by-one. It just makes good sense to reuse the same buffer, something already done in usei as well Besides the good sense, it helps to discard some memory corruption theories(if we do not allocate a different buffer for every single recv, it becomes harder for a bad buffer write to hit the recv buffer!)
Fix race condition in malicious_data_tests. A couple of test cases used asynchronous receive in a polling loop. This could lead to overlapping async operations, ultimately failing test case expectations. Replace the async polling loop with the same kind of synchronous receive that is already used in other test cases in the same file.
Enforce single dispatch for big_payload tests. All of these tests expect messages to be delivered in a specific order. Under load, the default settings are likely to lead to spawning a second dispatcher thread, which then defies test case expectations leading to spurious failures. Instead of refactoring all the tests to properly handle out-of-order delivery, change configs to enforce a single dispatcher thread. The higher timeout just avoids useless "BLOCKING CALL" messages if the default of 100 ms is exceeded.
Fix offered_services_info_test. This test sporadically failed, because test expectations would count the "pseudo-availability" of [ffff.ffff] as one of the expected offered services if it arrived early enough. Filter this out to fix the test.
The usage of the timer utility allows to drop the mutex from the rmc, and the periodic look-up of the configuration. On top it guarantees that the logging functions are only invoked, while the rmc is guaranteed to not have been cleaned up.
Some is forgotten content from 4eb5d72, the rest is just leftovers
Cleanup pending event registration Previously, it could happen that if an event could not be added to the registration command, we would enter an endless loop until the send encountered an error. As such, fix this situation and cleanup the code.
Fix additional boardnet tests. The previous fix in 420e0be handled a race in one of the tests, but interface_down_after_successful_subscription has the same issue. Apply the same fix there. Also clear the availability record before setting interfaces down in both test cases just to be sure there are no remnants from the setup phase. Additionally, fix a race condition in yet another boardnet test.
Introduce test timeout scaling. When running under sanitizers, test cases execute much slower than normally. As a result, waiting times and timeouts defined in test cases may not be sufficient under such conditions. This will be especially visible once test cases run in parallel, limiting CI performance even more. In order to avoid false positive timeouts, introduce a test timeout scaling factor that can be set in CI (or locally) to scale relevant timeouts in tests. Modify a number of tests prone to suffer from slow execution to use this in more or less obvious places. Additional uses of the scaling parameter may have to be introduced based on real-world results in subsequent commits.
Make debounce_filter_test more robust under load. This test still suffers from sporadic failures under load. A recent change increased tolerance of expectations for the measured intervals between messages, but it turned out to be sometimes not enough. Further increasing jitter tolerance would defeat the purpose of the test. Instead, observation shows that the average interval seems to be skewed by a burst of messages right at the start of the test; that is likely due to the way Valgrind schedules threads that may lead to messages piling up on the dispatcher side. Ignore those first few messages before starting to measure, once things have settled a bit.
duartenfonseca
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.