diff --git a/score/datarouter/BUILD b/score/datarouter/BUILD index 8747689d..cc90b047 100644 --- a/score/datarouter/BUILD +++ b/score/datarouter/BUILD @@ -208,8 +208,8 @@ cc_library( ":datarouter_types", ":dltprotocol", ":logparser_interface", + "//score/mw/log", "//score/mw/log/detail/data_router/shared_memory:reader", - "@score_baselibs//score/mw/log", "@score_baselibs//score/mw/log/configuration:nvconfig", "@score_baselibs//score/static_reflection_with_serialization/serialization", ], @@ -383,7 +383,7 @@ cc_library( strip_include_prefix = "src/applications", visibility = ["//score/datarouter/test:__subpackages__"], deps = [ - "@score_baselibs//score/mw/log", + "//score/mw/log", ], ) @@ -400,7 +400,7 @@ cc_library( "@score_logging//score/datarouter:__subpackages__", ], deps = [ - "@score_baselibs//score/mw/log", + "//score/mw/log", ], ) @@ -420,10 +420,10 @@ cc_library( ":logparser_factory_interface", ":message_passing_server", ":unixdomain_server", + "//score/mw/log", "//score/mw/log/detail/data_router/shared_memory:reader", "@score_baselibs//score/concurrency:synchronized", "@score_baselibs//score/language/futurecpp", - "@score_baselibs//score/mw/log", ], ) @@ -444,8 +444,8 @@ cc_library( ":logparser_factory_interface", ":message_passing_server", ":unixdomain_mock", + "//score/mw/log", "@score_baselibs//score/concurrency:synchronized", - "@score_baselibs//score/mw/log", ], ) @@ -467,8 +467,8 @@ cc_library( ":logparser_testing", ":message_passing_server", ":unixdomain_mock", + "//score/mw/log", "@score_baselibs//score/concurrency:synchronized", - "@score_baselibs//score/mw/log", ], ) @@ -627,8 +627,8 @@ cc_library( ":unixdomain_server", "//score/datarouter/network:vlan", "//score/datarouter/src/persistent_logging/persistent_logging_stub:sysedr_stub", + "//score/mw/log", "@score_baselibs//score/language/futurecpp", - "@score_baselibs//score/mw/log", "@score_baselibs//score/mw/log/configuration:nvconfig", "@score_baselibs//score/os:socket", "@score_baselibs//score/os:stat", @@ -656,7 +656,7 @@ cc_library( ], deps = [ ":dltserver_common", - "@score_baselibs//score/mw/log", + "//score/mw/log", "@score_baselibs//score/mw/log/configuration:nvconfig", "@score_baselibs//score/os:socket", "@score_baselibs//score/os:stat", @@ -716,8 +716,8 @@ cc_library( ":logchannel_utility", ":socketserver_config_helpers", "//score/datarouter/src/persistency:interface", + "//score/mw/log", "@rapidjson", - "@score_baselibs//score/mw/log", ], ) @@ -866,7 +866,7 @@ cc_library( visibility = ["//score/datarouter/test:__subpackages__"], deps = [ ":datarouter_options", - "@score_baselibs//score/mw/log", + "//score/mw/log", ] + deps, ) for (name, deps, test_only) in [ @@ -1109,7 +1109,7 @@ cc_library( "@score_baselibs//score/os:stdio", "@score_baselibs//score/os/utils:path", "@score_baselibs//score/os/utils:thread", - "@score_baselibs//score/mw/log", + "//score/mw/log", "@score_baselibs//score/mw/log/configuration:nvconfig", "//score/datarouter/dlt_filetransfer_trigger_lib:filetransfer_message_types", "//score/datarouter/src/persistent_logging/persistent_logging_stub:sysedr_stub", diff --git a/score/datarouter/doc/design/logging_architecture.md b/score/datarouter/doc/design/logging_architecture.md index d24c00b6..268a27da 100644 --- a/score/datarouter/doc/design/logging_architecture.md +++ b/score/datarouter/doc/design/logging_architecture.md @@ -116,7 +116,6 @@ This construct uses the `log_entry` singleton template to register type informat The diagrams below illustrate the high-level class structure of logging framework components. The [ara::log][1] implementation conforms to Adaptive AUTOSAR specification R1903. -![alt text][package-ara-log] ![alt text][package-datarouter] @@ -135,7 +134,6 @@ The activity diagram below depicts the first-run process: ### ara::log implementation The Adaptive AUTOSAR logging interface implementation follows standard specifications. The system creates LogStream objects dynamically to enable isolated collection of log message items and atomic message commits on stream flush. -![alt text][seq-ara-log] ![alt text][log-filtering-client-end] ### Ring buffer and linear allocator buffer @@ -143,13 +141,15 @@ The Adaptive AUTOSAR logging interface implementation follows standard specifica The ring buffer operates in shared memory to minimize copy overhead. Shared memory IPC provides optimal speed and flexibility for this implementation. -### Message passing interaction +### Datarouter-Client Session -The message passing connection transmits initial connection information, notifications, and handles disconnections. The connection lifecycle proceeds as follows: +The Datarouter-Client Session uses [message_passing](https://github.com/eclipse-score/communication/tree/main/score/message_passing) IPC for the initial connection, buffer acquire requests, notifications, and disconnections. The `DataRouterRecorder` sets up the session when it is created and closes it when it is destroyed. In between, the datarouter keeps one `IServerConnection` handle per client and drives the log acquisition from the server side. -- The `DataRouterRecorder` constructor creates a `DatarouterMessageClient` instance, spawning a thread that attempts server connections every 100ms. -- Upon successful connection, the client transmits a message containing application information to the server. -- The `DataRouterRecorder` destructor notifies the thread, causing normal termination. +The main idea is to keep clients independent, so that one non-responsive client should not stall the datarouter. This matters because the datarouter serves all clients from a single thread, one after another on each periodic tick, so a single blocking call would hold up every other client. To avoid this, the datarouter asks for buffers (to read) using non-blocking QNX pulses (`IServerConnection::Notify()`), which return right away and never wait on the client. So if a client is slow or stuck, its pulse simply stays unanswered while the healthy clients keep getting served. See [session_sequence_diagram](uml/client_session_interaction_sequence.puml) for the full flow. + +However, with such a design there are risks of stale sessions. A small per-session watchdog limits how long the datarouter waits for an acquire response. If a client keeps missing its deadline, the watchdog cleans up the stale session, but first it does a best-effort read of the client's last buffer so no logs are lost unnecessarily. See [activity_watchdog_session](uml/activity_watchdog_session_lifecycle.puml). + +The setup also handles an early-disconnect race, wherein if a client crashes while the datarouter is still building its session, the connect and disconnect paths work together to throw away the half-built session instead of keeping a dangling connection pointer. The client can then reconnect cleanly afterwards. See [early_disconnect_race](uml/sequence_early_disconnect_race.puml). ### datarouter @@ -171,9 +171,7 @@ The datarouter requires two configuration files: [context-ecu]: uml/context-ecu.png "Context: logging framework in xPAD ECU (hPAD example)" [context-highlevel]: uml/context-highlevel.png "Implementation details: general approach" -[seq-ara-log]: uml/seq-ara-log.png "ara::log call conversion to libtracing" [seq-trace]: uml/seq-trace.png "Activity diagram for tracing functionality" -[package-ara-log]: uml/package-ara-log.png "Package contents for ara::log" [package-datarouter]: uml/package-datarouter.png "Package contents for datarouter" [log-filtering-client-end]: uml/dlt_message_filtering_frontend.png "DLT log filtering in the frontend (client side)" [log-filtering-datarouter]: uml/dlt_message_filtering_backend.png "DLT log filtering in the backend (Datarouter)" diff --git a/score/datarouter/doc/design/uml/activity_watchdog_session_lifecycle.puml b/score/datarouter/doc/design/uml/activity_watchdog_session_lifecycle.puml new file mode 100644 index 00000000..e297fced --- /dev/null +++ b/score/datarouter/doc/design/uml/activity_watchdog_session_lifecycle.puml @@ -0,0 +1,41 @@ +@startuml activity_watchdog_lifecycle + +title Per-Session Acquire Watchdog Lifecycle + +start + +:mp_worker tick fires; + +if (Session has acquire_in_flight?) then (yes) + if (now() > acquire_deadline?) then (yes) + :Clear acquire_in_flight; + :Increment acquire_miss_count; + if (acquire_miss_count >= max_misses?) then (yes) + :Tear down session; + stop + else (no) + :Allow next tick to\nretry acquire; + endif + else (no) + :Wait for response\nor deadline expiry; + endif +else (no) + :connection.Notify(kAcquireRequest); + :Set acquire_in_flight = true; + :Set acquire_deadline =\nnow() + watchdog_config.deadline; +endif + +:Wait for next tick; + +if (kAcquireResponse received?) then (yes) + :Clear acquire_in_flight; + :Reset acquire_deadline; + :Reset acquire_miss_count = 0; + :Process acquired buffer; +else (no) + :Continue waiting; +endif + +stop + +@enduml diff --git a/score/datarouter/doc/design/uml/client_session_interaction_sequence.puml b/score/datarouter/doc/design/uml/client_session_interaction_sequence.puml new file mode 100644 index 00000000..3f5d439a --- /dev/null +++ b/score/datarouter/doc/design/uml/client_session_interaction_sequence.puml @@ -0,0 +1,171 @@ +@startuml sequence_notify_acquire_request + +title IServerConnection::Notify() + Watchdog\n(Non-blocking MsgDeliverEvent) + +skinparam sequenceMessageAlign center +skinparam responseMessageBelowArrow true + +box "Client A (healthy)" #LightBlue + participant "User Threads\n(Client A)" as UTA + participant "SharedMemoryWriter\n(Client A)" as SHMA + participant "QnxDispatchEngine\n(Client A)" as AE +end box + +box "Client X (unhealthy)" #PeachPuff + participant "QnxDispatchEngine\n(Client X)" as XE +end box + +box "Datarouter Process" #LightGreen + participant "QnxDispatchEngine\n(server engine)" as SE + participant "mp_worker thread" as MW + participant "Watchdog Timer" as WD + participant "DLT output" as DLT +end box + +note over SE + Datarouter sends kAcquireRequest + via **IServerConnection::Notify()** on the already- + established server connection for each client. + + Notify() calls **MsgDeliverEvent()** which delivers + a QNX pulse to the client — **fire-and-forget, + returns immediately, never blocks**. +end note + +== Initialization == + +UTA -> AE : DatarouterMessageClientImpl::Run() +AE -> SE : kConnect (appid, uid, pid)\n[Send() — one-time, acceptable] + +SE -> SE : connect_callback()\nStore IServerConnection\nfor this client + +note over SE + Datarouter now holds the + IServerConnection reference + for each connected client. + This is the handle for Notify(). +end note + +MW -> MW : Create session + start watchdog\n(per-client deadline timer) + +== Steady-State: Acquire via Notify == + +MW -> MW : RunWorkerThread()\ntick() fires for Client A + +MW -> SE : **connection_A.Notify(kAcquireRequest)** + +SE -> SE : QnxDispatchServer::\nServerConnection::Notify() + +note over SE #LightGreen + Internally: + 1. Copy message to notify_pool_ slot + 2. Push to send_queue_ + 3. **MsgDeliverEvent(rcvid, &pulse)** - Returns immediately and never blocks. +end note + +SE -->> AE : QNX pulse delivered\n(async, non-blocking) + +AE -> AE : dispatch_block() receives pulse\n→ readv() → ProcessInput(NOTIFY)\n→ notify_callback_() + +note over AE + Client's NotifyCallback + (new, registered at Start()): + validates message, calls + ReadAcquire(), sends response. +end note + +AE -> SHMA : ReadAcquire() +SHMA -> SHMA : alternating_reader_.Switch()\nBuffer A → B + +AE -> SE : **Send(kAcquireResponse + result)**\n[Client → Datarouter: Send is safe,\nDatarouter is always ready to receive] + +SE -> MW : MessageCallback()\n→ OnAcquireResponse() +MW -> MW : session.on_acquire_response(acq) + +MW -> SHMA : Wait for writers to release\nRead acquired Buffer A +MW -> DLT : Forward log records ✓ + +MW -> WD : Reset watchdog timer for Client A + +== Client X is unhealthy == + +note over XE #PeachPuff + X's engine thread is unhealthy (probable causes): + • crash leading to suspended thread + • Priority inversion + • CPU starvation +end note + +MW -> MW : tick() fires for Client X +MW -> SE : **connection_XYZ.Notify(kAcquireRequest)** + +SE -> SE : MsgDeliverEvent()\n→ **returns immediately!** + +note over SE #LightGreen + Pulse is queued in X's + QNX receive channel. + Datarouter does NOT block. + Server engine thread is free. +end note + +note over XE #PeachPuff + X never processes the pulse. + No response comes back. + **But Datarouter is NOT blocked.** +end note + +== Meanwhile: Client A continues unaffected == + +MW -> MW : tick() fires for Client A +MW -> SE : **connection_A.Notify(kAcquireRequest)** +SE -->> AE : pulse delivered +AE -> SHMA : ReadAcquire() +AE -> SE : Send(kAcquireResponse) +MW -> DLT : Forward Client A's logs + +note over DLT #LightGreen + **Client A is unaffected** + by X being unhealthy. +end note + +== Watchdog: Detect unresponsive X == + +WD -> WD : Client X deadline expired\n(no kAcquireResponse within T seconds) + +WD -> MW : Notify: X unresponsive + +MW -> MW : Increment X miss counter + +alt X misses < max_allowed_misses + MW -> MW : Retry: enqueue next tick\n(X gets another chance) + MW -> SE : connection_XYZ.Notify(kAcquireRequest)\n[still non-blocking, still safe] +else X misses >= max_allowed_misses + MW -> MW : **Tear down X session** + + note over MW #LightYellow + Session teardown: + 1. Mark session for delete + 2. Attempt post-mortem read + of X's current buffer + (best-effort, no IPC needed) + 3. Remove from pid_session_map_ + 4. Log diagnostic event + end note + + MW -> SHMA : Post-mortem: read whatever\nis in X's last buffer\n(shmem is still mapped) + MW -> DLT : Forward recovered records\n(if any valid ones found) + + MW -> MW : Erase session from map\nClose server connection + destroy XE +end + +== Subsequent X reconnect (if X recovers) == + +note over MW + If X was merely slow (not dead), + it may reconnect later with a new + kConnect. Datarouter creates a + fresh session — clean slate. +end note + +@enduml diff --git a/score/datarouter/doc/design/uml/context-details.png b/score/datarouter/doc/design/uml/context-details.png new file mode 100644 index 00000000..e57e01c3 Binary files /dev/null and b/score/datarouter/doc/design/uml/context-details.png differ diff --git a/score/datarouter/doc/design/uml/context-details.puml b/score/datarouter/doc/design/uml/context-details.puml new file mode 100644 index 00000000..d556a9a4 --- /dev/null +++ b/score/datarouter/doc/design/uml/context-details.puml @@ -0,0 +1,135 @@ +@startuml + +skinparam component { + backgroundColor<> Aquamarine + backgroundColor<> DarkCyan + fontColor<> White +} + +skinparam queue { + backgroundColor<> Aquamarine +} + +frame { + node AdaptiveApplication { + component ApplicationLogic + interface "mw::log" as aralog1 + component "mw::log implementation" <> as aralog1_impl + component rapidjson <> as rj1 + + aralog1 - aralog1_impl + ApplicationLogic --> aralog1 : verbose + ApplicationLogic --> aralog1 : non-verbose + aralog1_impl .> rj1 : use + } + + queue Ringbuffer1 <> as RB1 + database "AdaptiveApplication/logging.json" as cfg1 + rj1 -> cfg1 +} + + +frame { + node SystemApplication { + component SystemApplicationLogic + interface "mw::log" as aralog2 + component "mw::log implementation" <> as aralog2_impl + component rapidjson <> as rj2 + + aralog2 - aralog2_impl + SystemApplicationLogic --> aralog2 : verbose + aralog2_impl .> rj2 : use + } + + queue Ringbuffer2 <> as RB2 + database "SystemApplication/logging.json" as cfg2 + rj2 -> cfg2 +} + +note right of RB1 + Shared memory + allocated by AdaptiveApplication +end note + +aralog1_impl ..> RB1 : own +aralog2_impl ..> RB2 : own +aralog1_impl --> RB1 : write +aralog2_impl --> RB2 : write + +database "MessageIDs" as type_id + +node "Datarouter"{ + + component [Source Session 1] <> as sourcesession1 + component [Source Session 2] <> as sourcesession2 + + + component "NonVerboseHandler" as nvh { + component "Filter" as nvfilter + component "Pack" as nvpack + + type_id <.. nvfilter + type_id <.. nvpack + nvfilter --> nvpack + } + + RB1 <-- sourcesession1 : read + RB2 <-- sourcesession2 : read + + component "VerboseHandler" as vh { + component "Pack" as vpack + component "Filter" as vfilter + + vfilter --> vpack + } + + component "DLT Output" as dlt_out { + component "Dispatch" as Demux + component "Traffic shaping queues" as out { + queue "High Priority DLT" as dlt1 + queue "Low Priority DLT" as dlt2 + queue "Raw data" as dlt3 + } + Demux -> dlt1 + Demux -> dlt2 + Demux -> dlt3 + } + + component "Type T Handler" as type_handler { + } +} + +queue "Subscriber Buffer" as sub_rb +note right of sub_rb + Shared memory + allocated by Datarouter +end note + + +node TypeXSubscriber as type_subscriber { +} + +type_forwarder ==> sub_rb +sub_rb ==> type_subscriber + +frame "UDP ports" { + boundary HighPriorityMessages as udp_1 + boundary LowPriorityMessages as udp_2 + boundary RawData as udp_3 +} + +sourcesession1 ~~> vfilter: +sourcesession2 ~~> vfilter: +sourcesession1 ..> nvfilter: * +sourcesession2 ..> nvfilter: * +sourcesession1 --> type_handler: +sourcesession2 --> type_handler: + +vpack -> Demux +nvpack -> Demux + +dlt1 -> udp_1 +dlt2 -> udp_2 +dlt3 -> udp_3 + +@enduml diff --git a/score/datarouter/doc/design/uml/context-ecu.png b/score/datarouter/doc/design/uml/context-ecu.png new file mode 100644 index 00000000..73df3ed7 Binary files /dev/null and b/score/datarouter/doc/design/uml/context-ecu.png differ diff --git a/score/datarouter/doc/design/uml/context-ecu.puml b/score/datarouter/doc/design/uml/context-ecu.puml new file mode 100644 index 00000000..32d56df3 --- /dev/null +++ b/score/datarouter/doc/design/uml/context-ecu.puml @@ -0,0 +1,23 @@ +@startuml +title Logging context in Safe-Posix-Platform (SPP) +frame SPP { + + frame "Adaptive AUTOSAR CPU" { + node Application1 + node Application2 + node SystemApplication + node "Logging framework" as DataRouter + boundary "UDP multicast" as UDP + + Application1 ..> DataRouter : LOG() + Application2 ..> DataRouter : TRACE() + SystemApplication ..> DataRouter : LOG() + DataRouter ..> UDP: DLT + } +} + +node Logger + +UDP ==> Logger + +@enduml diff --git a/score/datarouter/doc/design/uml/context-highlevel.png b/score/datarouter/doc/design/uml/context-highlevel.png new file mode 100644 index 00000000..8c9f38be Binary files /dev/null and b/score/datarouter/doc/design/uml/context-highlevel.png differ diff --git a/score/datarouter/doc/design/uml/context-highlevel.puml b/score/datarouter/doc/design/uml/context-highlevel.puml new file mode 100644 index 00000000..4d57073e --- /dev/null +++ b/score/datarouter/doc/design/uml/context-highlevel.puml @@ -0,0 +1,52 @@ +@startuml + +title Implementation approach + +frame "Adaptive AUTOSAR ECU" { + node AdaptiveApplication { + component ApplicationLogic + interface "mw::log" as aralog1 + component "mw::log implementation" as aralog1_impl + + aralog1 - aralog1_impl + ApplicationLogic --> aralog1 : verbose + ApplicationLogic --> aralog1 : non-verbose + + } + note right of AdaptiveApplication + EV Application, + e.g., RoadModel + end note + + + + node SystemApplication { + component SystemApplicationLogic + interface "mw::log" as aralog2 + component "mw::log implementation" as aralog2_impl + + aralog2 - aralog2_impl + SystemApplicationLogic --> aralog2 : verbose + } + note right of SystemApplication + e.g. SOME/IP daemon, + ABAC component, etc. + end note + + + node Datarouter { + } + note right of Datarouter + central logging daemon + end note +} + +boundary "UDP Multicast" as udp + +aralog1_impl --> Datarouter : write +aralog2_impl --> Datarouter : write + +Datarouter --> udp: "send DLT messages:" + + +@enduml diff --git a/score/datarouter/doc/design/uml/context-sw-units.png b/score/datarouter/doc/design/uml/context-sw-units.png new file mode 100644 index 00000000..b87698f3 Binary files /dev/null and b/score/datarouter/doc/design/uml/context-sw-units.png differ diff --git a/score/datarouter/doc/design/uml/context-sw-units.puml b/score/datarouter/doc/design/uml/context-sw-units.puml new file mode 100644 index 00000000..e5bb643d --- /dev/null +++ b/score/datarouter/doc/design/uml/context-sw-units.puml @@ -0,0 +1,71 @@ +@startuml + +frame ApplicationProcess { + node AdaptiveApplication { + component ApplicationLogic + interface "ara::log" <> as aralog + component "ara::log implementation" as a1 { + component LogManager + component Logger + component LogStream + } + component "score::mw::log" { + component log_entry + component "logging_serializer" as serializer + component logger { + component SharedMemoryWriter + } + + component MessagePassingClient + SharedMemoryWriter ..> MessagePassingClient : notify + + interface TRACE <> + + TRACE -> log_entry + log_entry --> serializer : TryWriteIntoSharedMemory() + serializer --> SharedMemoryWriter : AllocAndWrite() + log_entry --> logger : instance() + + } + + component "Configuration" { + component "score::mw::log::detail::Configuration" as LogConfig + component "JSON Parser\n(rapidjson)" as ConfigReader + } + + component rapidjson + ConfigReader ..> LogConfig + ConfigReader -- rapidjson + + aralog -> LogManager + + LogManager ~> Logger + Logger ~> LogStream + + aralog -> LogStream + + LogStream -> TRACE + ApplicationLogic --> aralog : verbose + ApplicationLogic --> TRACE : non-verbose + a1 ..> "score::mw::log" : use + } + + queue "Shared Memory Buffer" as RB +} + +node "Datarouter"{ + component MessagePassingServer + component "Source Session" as session { + component SharedMemoryReader + } + + MessagePassingServer ...> session +} + +MessagePassingClient ..> MessagePassingServer: connect +MessagePassingClient -> session: ping + +SharedMemoryWriter .> RB : write +RB <. SharedMemoryReader : read + +@enduml diff --git a/score/datarouter/doc/design/uml/detail-ipc.png b/score/datarouter/doc/design/uml/detail-ipc.png new file mode 100644 index 00000000..cea9dd60 Binary files /dev/null and b/score/datarouter/doc/design/uml/detail-ipc.png differ diff --git a/score/datarouter/doc/design/uml/detail-ipc.puml b/score/datarouter/doc/design/uml/detail-ipc.puml new file mode 100644 index 00000000..79cba155 --- /dev/null +++ b/score/datarouter/doc/design/uml/detail-ipc.puml @@ -0,0 +1,32 @@ +@startuml + +node AdaptiveApplication { + component "Application" as app +} + +node "Datarouter" { + component "Message Server" as msg_server + component [Source Session] as sourcesession + component UnixDomainServer +} + +node "DatarouterConf" { + component UnixDomainClient as config_client +} + +queue "SourceBuffer: MWSR buffer" as RB1 +note top of RB1 + Shared memory + allocated by Application +end note + +app -> msg_server : connect() +config_client -> UnixDomainServer : connect() + +app .> RB1 : create() +app -> RB1 : write() + +RB1 <- sourcesession : read() +msg_server ..> sourcesession: create() + +@enduml diff --git a/score/datarouter/doc/design/uml/dlt_message_filtering_backend.png b/score/datarouter/doc/design/uml/dlt_message_filtering_backend.png new file mode 100644 index 00000000..0c2a45e4 Binary files /dev/null and b/score/datarouter/doc/design/uml/dlt_message_filtering_backend.png differ diff --git a/score/datarouter/doc/design/uml/dlt_message_filtering_backend.puml b/score/datarouter/doc/design/uml/dlt_message_filtering_backend.puml new file mode 100644 index 00000000..a65da374 --- /dev/null +++ b/score/datarouter/doc/design/uml/dlt_message_filtering_backend.puml @@ -0,0 +1,50 @@ +@startuml +title log message filtering based on log_level (Datarouter - Backend) + +start + +NOTE: This Flow diagram excludes the sequence for filtering DLT logs based on diagnostic job requests + +:log_level_threshold = kVerbose (Default; hardcoded in socketserver_config.h); +:log_level = kInfo (Default; hardcoded); + + if (Parse class-id.json file (message ID table) success) then (yes) + note left + The class-id.json file is an output of the dlt_create_fibex generator + containing unique message ids mapped to app_id, ctx_id and log_level pairs + for message Types that needs to be traced out + end note + :Register message based on NvMsgDescriptor (message_id, app_id, ctx_id, log_level); + NOTE: class-id.json + :log_level = log_level for app_id ctx_id pair; + note left + The log_level is picked based on the inputs for the dlt_fibex_defenition + generator for the respective Type that needs to be traced. + E.g.: *_fibex.cpp + end note + else (no) + endif + +if (log-channels.json file found?) then (yes) +else (no) + stop +endif + +: Parse log-channels.json; +NOTE: log-channels.json + +if (message thresholds set in log-channels.json?) then (yes) + :log_level_threshold = message_threshold (log_level) for the respective ctx_id; +else (no) + endif + +if (log_level > logLevel_threshold) then (yes) + :Drop the log message + (Will NOT copy the DLT message + to io-pkt buffers); +else (no) + :Copy the DLT message to io-pkt buffers; +endif + +stop +@enduml diff --git a/score/datarouter/doc/design/uml/dlt_message_filtering_frontend.png b/score/datarouter/doc/design/uml/dlt_message_filtering_frontend.png new file mode 100644 index 00000000..0a40244b Binary files /dev/null and b/score/datarouter/doc/design/uml/dlt_message_filtering_frontend.png differ diff --git a/score/datarouter/doc/design/uml/dlt_message_filtering_frontend.puml b/score/datarouter/doc/design/uml/dlt_message_filtering_frontend.puml new file mode 100644 index 00000000..44a667c3 --- /dev/null +++ b/score/datarouter/doc/design/uml/dlt_message_filtering_frontend.puml @@ -0,0 +1,47 @@ +@startuml +title log message filtering based on log_level (client - Frontend) + +start + +if (logging.json file is found) then (yes) + NOTE: logging.json + :log_level is updated; +else (no) + if (ecu_logging_config.json is found) then (yes) + :log_level is updated; + else (no) + :log_level = kWarn (default; hardcoded); + endif + NOTE: ecu_logging_config.json (fallback if logging.json is NOT found) +endif + +if (Logging) then (Verbose logging) + :log_level_threshold = kWarn (Default); + :log_level_threshold = log_level based on logging.json OR + if extracted ContextConfigs (ctx_id and log_level pairs); +else (Non-Verbose logging) + :log_level_threshold = kVerbose (default); + if (class-id.json file (message table) is found) then (yes) + NOTE: class-id.json + :Parse class-id.json file (message-ID table); + :Register type based on NvMsgDescriptor (message_id, app_id, ctx_id, log_level); + if (context configs specified in the logging.json file?) then (yes) + NOTE: logging.json + :log_level_threshold = log_level for the respective ctx_id; + else (no) + endif + else (no) + endif +endif + +if (log_level > logLevel_threshold) then (yes) + :Drop the log message + - Will NOT write the message into logging buffers; +else (no) + :Write the log message into logging buffers; +endif + +NOTE: logging buffers are maintained in shared_memory (shared between client and datarouter) + +stop +@enduml diff --git a/score/datarouter/doc/design/uml/file_transfer_feature_class_diagram.png b/score/datarouter/doc/design/uml/file_transfer_feature_class_diagram.png new file mode 100644 index 00000000..c3a22b78 Binary files /dev/null and b/score/datarouter/doc/design/uml/file_transfer_feature_class_diagram.png differ diff --git a/score/datarouter/doc/design/uml/file_transfer_feature_class_diagram.puml b/score/datarouter/doc/design/uml/file_transfer_feature_class_diagram.puml new file mode 100644 index 00000000..e9a52c26 --- /dev/null +++ b/score/datarouter/doc/design/uml/file_transfer_feature_class_diagram.puml @@ -0,0 +1,143 @@ +@startuml file_transfer_feature_class_diagram +title File Transfer Feature - Class Diagram + +interface "LogParser::TypeHandler" as TypeHandler { + + handle(timestamp_t, const char*, bufsize_t): void +} + +class StubFileTransferStreamHandler { + - _output: IOutput& + -- + + StubFileTransferStreamHandler(IOutput& output) + + handle(timestamp_t, const char*, bufsize_t): void +} + +interface "StubFileTransferStreamHandler::IOutput" as StubIOutput { + + sendFTVerbose(score::cpp::span, + mw::log::LogLevel, dltid_t, dltid_t, + uint8_t, uint32_t): void +} + +class FileTransferStreamHandler { + - output_: IOutput& + - filetransfer_thread: std::thread + - exit_requested: std::atomic_bool + - filetransfer_mutex: std::shared_timed_mutex + - container: std::queue + - readfile: std::string + - creationdate: std::string + - appid: dltid_t + - ctxid: dltid_t + - serialno_: std::uint32_t + - fsize: std::uint32_t + - packagecount: std::uint32_t + -- + + FileTransferStreamHandler(IOutput& output) + + ~FileTransferStreamHandler() + + handle(timestamp_t, const char*, bufsize_t): void + - ProcessFileTransfer(): void + - ReadFileHeaderInfo(const std::string&): bool + - LogFileHeader(): bool + - LogFileData(): void + - LogFileEnd(): void + - LogFileError(int16_t, std::string): void +} + +interface "FileTransferStreamHandler::IOutput" as FileIOutput { + + sendFTVerbose(score::cpp::span, + mw::log::LogLevel, dltid_t, dltid_t, + uint8_t, uint32_t): void +} + +class "dltserver::Output (Mock)" as MockOutput { + + ~Output() + + MOCK_METHOD(sendFTVerbose, + (score::cpp::span, + mw::log::LogLevel, dltid_t, dltid_t, + uint8_t, uint32_t), (override)) +} + +class "dltserver::Output" as Output { + + ~Output() + + sendFTVerbose(score::cpp::span, + mw::log::LogLevel, dltid_t, dltid_t, + uint8_t, uint32_t): void +} + +class "FileTransferHandlerFactory" as Factory { + + create(): std::unique_ptr + # FileTransferHandlerFactory() = default +} + +class StubFileTransferHandlerFactory { + - mock_output_: Output& + -- + + StubFileTransferHandlerFactory(Output& mock_output) + + createConcreteHandler(): std::unique_ptr +} + +class FileTransferStreamHandlerFactory { + - output_: Output& + -- + + FileTransferStreamHandlerFactory(Output& output) + + createConcreteHandler(): std::unique_ptr +} + +class DltLogServer { + + createFileTransferHandler(): + std::unique_ptr +} + +class "score::logging::FileTransferEntry" as FileTransferEntry { + + appid: dltid_t + + ctxid: dltid_t + + file_name: std::string + + delete_file: uint8_t +} + +interface "score::logging::IFileTransfer" as IFileTransfer { + + TransferFile(const std::string& file_name, + const bool delete_file): void +} + +class "score::logging::FileTransfer" as FileTransfer { + - appid_: std::string + - ctxid_: std::string + -- + + FileTransfer(const std::string& appid, + const std::string& ctxid) + + TransferFile(const std::string&, bool): void +} + +note bottom of StubFileTransferHandlerFactory +#if defined(ENABLE_FILETRANSFER_FEATURE) + using HandlerType = FileTransferStreamHandlerFactory; +#else + using HandlerType = StubFileTransferHandlerFactory; +#endif +end note + +TypeHandler <|.. StubFileTransferStreamHandler +TypeHandler <|.. FileTransferStreamHandler + +StubFileTransferStreamHandler ..> StubIOutput +FileTransferStreamHandler ..> FileIOutput + +StubIOutput <|.. MockOutput +FileIOutput <|.. Output + +Factory <|-- StubFileTransferHandlerFactory +Factory <|-- FileTransferStreamHandlerFactory + +StubFileTransferHandlerFactory ..> StubFileTransferStreamHandler : <> +FileTransferStreamHandlerFactory ..> FileTransferStreamHandler : <> + +DltLogServer ..> Factory : uses factory +Output ..> Factory : injects + +FileTransferStreamHandler *-- "queue" FileTransferEntry + +IFileTransfer <|.. FileTransfer +FileTransfer ..> FileTransferEntry : <> + +@enduml diff --git a/score/datarouter/doc/design/uml/ipc_status_quo.puml b/score/datarouter/doc/design/uml/ipc_status_quo.puml new file mode 100644 index 00000000..b0b41523 --- /dev/null +++ b/score/datarouter/doc/design/uml/ipc_status_quo.puml @@ -0,0 +1,129 @@ +@startuml IPC Status Quo - Datarouter Communication +title ARA Logging IPC Architecture\nLogging Client ↔ Datarouter Communication + +!theme plain +skinparam BoxPadding 10 +skinparam ParticipantPadding 20 +skinparam SequenceMessageAlignment center + +actor "Logging Client" as Client +participant "Shared Memory\n(ring buffer)" as SHM +participant "IPC Channel\n(Message Passing)" as IPC +participant "Datarouter" as DR + +== Initialization Phase == +activate Client +activate SHM + +Client -> IPC: **2. Connect to Datarouter** +activate IPC +note right + Platform-specific IPC: + - Linux: Unix Domain Socket + - QNX: QNX Message Passing +end note + +IPC -> DR: **3. Connection established** +activate DR + +Client -> IPC: **4. Send ConnectMessage** +note right + Metadata includes: + - AppID, UID, PID + - Shared memory filename + - Dynamic identifier flag +end note + +IPC -> DR: ConnectMessage received + +DR -> SHM: **5. Open & map shared memory** +note right + Datarouter opens client's file + Maps same memory region + Now both share ring buffer +end note + +SHM --> DR: Shared memory mapped +note left + Client can log immediately + Datarouter polls asynchronously + Fully decoupled +end note + +== Data Exchange Loop == + +note over Client, DR + **Ring Buffer Protocol** + Zero-copy via shared memory +end note + +Client -> SHM: **Write log messages** +note right + LOG_INFO() << "message" + TRACE(structure) + + Updates ring buffer: + - Write pointer advances + - No system calls +end note + +DR -> IPC: **Send kAcquireRequest (0x01)** +note right + Message: Single byte [0x01] + Datarouter polls for new data + Sent periodically (keepalive) +end note + +IPC --> Client: **received_send_message_callback** +note left + Callback triggered in client's + receiver when message arrives + + Internally handles: + - ReadAcquire() for buffer ID + - Unlink file (first time only) + - Send kAcquireResponse +end note + +Client -> IPC: **Send kAcquireResponse (0x02)** +note right + Contains ReadAcquireResult: + - acquired_buffer (0 or 1) + + Tells datarouter which + buffer to read from +end note + +IPC -> DR: AcquireResponse received + +DR -> SHM: **Read log messages directly** +note right + Zero-copy read from + shared memory buffer + + Parse log entries + Route to DLT channels + Apply quotas/filters +end note + +note over Client, DR + **Next kAcquireRequest implicitly releases previous buffer** + No explicit release message in protocol +end note + +== Shutdown == + +Client ->x IPC: Connection closed +deactivate IPC +IPC -> DR: **on_closed_by_peer()** +note right + Drain remaining logs + Cleanup session +end note + +DR ->x SHM: **munmap()** - Unmap +deactivate DR +deactivate SHM +deactivate Client + +@enduml diff --git a/score/datarouter/doc/design/uml/package-datarouter.png b/score/datarouter/doc/design/uml/package-datarouter.png new file mode 100644 index 00000000..d2245ced Binary files /dev/null and b/score/datarouter/doc/design/uml/package-datarouter.png differ diff --git a/score/datarouter/doc/design/uml/package-datarouter.puml b/score/datarouter/doc/design/uml/package-datarouter.puml new file mode 100644 index 00000000..60eb030c --- /dev/null +++ b/score/datarouter/doc/design/uml/package-datarouter.puml @@ -0,0 +1,38 @@ +@startuml + +title Datarouter Packages + +package datarouter { + package Datarouter { + class synchronized + class DataRouter + class DataRouter::SourceSession + class SocketServer + class IPersistentDictionary + class PersistentDictionaryFactory + class AraPerPersistentDictionaryFactory + class AraPerPersistentDictionary + class StubPersistentDictionaryFactory + class StubPersistentDictionary + } + + package internal { + class LogParser + class LogParser::AnyHandler + class LogParser::TypeHandler + class MessagePassingServer::ISession + class EmptySysedrFactory + class EmptySysedrHandler + class ISysedrHandler + class LogEntryFactory + class SysedrConcreteFactory + class SysedrFactory + class SysedrHandler + class UnixDomainClient + class UnixDomainSockAddr + class UnixDomainServer::SessionHandle + class UnixDomainServer::SessionWrapper + } +} + +@enduml diff --git a/score/datarouter/doc/design/uml/persistent-cleanup-cls-dgm.puml b/score/datarouter/doc/design/uml/persistent-cleanup-cls-dgm.puml new file mode 100644 index 00000000..fd37f6d5 --- /dev/null +++ b/score/datarouter/doc/design/uml/persistent-cleanup-cls-dgm.puml @@ -0,0 +1,43 @@ +@startuml persistent_cleanup_updated +title Persistent Logging Cleanup Integration + +interface GenericDataIdentifier { + +{abstract} Read(data_identifier, meta_info, cancellation_handler): Future + +{abstract} Write(data_identifier, request_data, meta_info, cancellation_handler): Future +} + +interface IVersionChangeFileCleaner { + +{abstract} DoFileCleanUp(file_list): void +} + +class PersistentLogging { + +SerializedRead(): Result + +SerializedWrite(ByteVector): Result + -ConstructReadFileList(): string + -DoCleanUp(): void + -SendPersistentLogFiles(): void + -FetchListOfPersistentFiles(): vector + -ThreadMethod(): void + -version_chng_file_cleaner_: unique_ptr + -file_transfer_: FileTransfer + -file_list_: vector + -worker_thread_: jthread +} + +class VersionChangeFileCleaner { + +DoFileCleanUp(file_list): void + -ReadLocalSwVersion(local_sw_version): bool + -ReadEcuSwVersion(ecu_sw_version): bool + -StoreLocalSwVersion(new_sw_version): void + -DeleteExistingPersistentLogFiles(file_list): void + -LocalSwVersionDoesNotExists(): bool + -NotSameAsLocalSwVersion(ecu_sw_version): bool + -filesystem_: Filesystem + -ddad_version_reader_: unique_ptr +} + +GenericDataIdentifier <|.. PersistentLogging : implements +PersistentLogging *-- IVersionChangeFileCleaner : owns +IVersionChangeFileCleaner <|.. VersionChangeFileCleaner : implements + +@enduml diff --git a/score/datarouter/doc/design/uml/persistent-cleanup-seq-dgm.puml b/score/datarouter/doc/design/uml/persistent-cleanup-seq-dgm.puml new file mode 100644 index 00000000..79d86661 --- /dev/null +++ b/score/datarouter/doc/design/uml/persistent-cleanup-seq-dgm.puml @@ -0,0 +1,130 @@ +@startuml persistent_cleanup_sequence +title Persistent Logging Cleanup Integration Sequence + +participant "DiagnosticClient" as DC +participant "PersistentLoggingService" as PLS +participant "DltLogServer" as DLS +participant "SysedrHandler" as SH +participant "FileCleanUpManager" as FCM +participant "DdadVersionReader" as DVR +participant "FileOperations" as FO +participant "ZlibCompressedFile" as ZF + +== Service Initialization == +PLS -> PLS : Initialize() +PLS -> PLS : InitializeDTCHandling() +PLS -> FCM : create(version_reader, file_operations) +FCM -> DVR : create() +FCM -> FO : create() + +== Version-Based Cleanup on Startup == +PLS -> FCM : CheckVersionChange() +FCM -> DVR : ReadEcuSwVersion() +DVR --> FCM : current_ecu_version + +FCM -> FO : FileExists(version_file_path) +FO --> FCM : file_exists_status + +alt version_file_exists + FCM -> FO : ReadFile(version_file_path) + FO --> FCM : stored_version, read_status + + alt read_success && stored_version != current_ecu_version + note right of FCM : Version changed - cleanup required + PLS -> PLS : FetchListOfPersistentFiles() + PLS -> SH : GetPersistentLogFileList() + SH --> PLS : persistent_file_list + + PLS -> FCM : DoCleanUp(persistent_file_list) + + loop for each file in persistent_file_list + FCM -> FO : RemoveFile(file_path) + FO --> FCM : removal_status + end + + FCM -> FO : WriteFile(version_file_path, current_ecu_version) + FO --> FCM : write_status + + else read_failed + FCM -> FCM : LogError("Failed to read version file") + end + +else version_file_not_exists + note right of FCM : First run - create version file + FCM -> FO : CreateFile(version_file_path) + FCM -> FO : WriteFile(version_file_path, current_ecu_version) + FCM -> FCM : LogInfo("Don't delete files on first run") +end + +== Diagnostic Service Offering == +PLS -> PLS : OfferService() +note right of PLS : Service available for diagnostic requests + +== Diagnostic Read Request (File List) == +DC -> PLS : Read(data_identifier, meta_info, cancellation_handler) +PLS -> PLS : FetchListOfPersistentFiles() +PLS -> SH : GetPersistentLogFileList() +SH --> PLS : file_list_string +PLS --> DC : OperationOutput(file_list) + +== Diagnostic Write Request (Trigger Cleanup) == +DC -> PLS : Write(data_identifier, request_data, meta_info, cancellation_handler) +PLS -> PLS : ProcessCleanupRequest(request_data) + +alt cleanup_all_files + PLS -> PLS : FetchListOfPersistentFiles() + PLS -> FCM : DoCleanUp(all_persistent_files) + + loop for each persistent file + FCM -> FO : RemoveFile(file_path) + FO --> FCM : removal_status + alt removal_failed + FCM -> FCM : LogError("Failed to remove file") + end + end + +else cleanup_by_age + PLS -> PLS : FilterFilesByAge(max_age_days) + PLS -> FCM : DoCleanUp(old_files) + + loop for each old file + FCM -> FO : RemoveFile(file_path) + end +end + +PLS -> PLS : CheckAndSetDtc() +note right of PLS : Set DTC if cleanup issues occurred + +== Integration with Active Logging == +note over SH, ZF +**During Active Persistent Logging:** +- SysedrHandler continues normal operation +- Files being written are protected from cleanup +- Cleanup only affects completed/closed files +end note + +SH -> ZF : write(dlt_data, size) +note right of SH : Active logging continues uninterrupted + +== Service Cleanup == +PLS -> FCM : ~FileCleanUpManager() +PLS -> PLS : UnInitialize() + +note over PLS, FCM +**Key Integration Points:** +- PersistentLoggingService manages both diagnostic access and cleanup +- FileCleanUpManager handles version-based and on-demand cleanup +- Integration with SysedrHandler for file status monitoring +- DTC handling for cleanup operation status +- Protection of active logging operations +end note + +note over DVR, FO +**Cleanup Strategy:** +- Version-based: Automatic cleanup on ECU software update +- On-demand: Diagnostic request-triggered cleanup +- Age-based: Remove files older than specified threshold +- Safe operation: Never interfere with active logging +end note + +@enduml diff --git a/score/datarouter/doc/design/uml/persistent_logging_class.puml b/score/datarouter/doc/design/uml/persistent_logging_class.puml new file mode 100644 index 00000000..819d107b --- /dev/null +++ b/score/datarouter/doc/design/uml/persistent_logging_class.puml @@ -0,0 +1,17 @@ +@startuml persistent_logging_class +title Persistent Logging Class Diagram + +SocketServer --> DltLogServer : creates +SocketServer --> DataRouter : creates +DataRouter *-- LogParser : owns +DltLogServer *-- DltNonverboseHandler : owns +DltLogServer *-- DltVerboseHandler : owns +DltLogServer *-- FileTransferStreamHandler : owns +DltLogServer *-- SysedrHandler : owns +DltLogServer --> LogParser : registers handlers +SysedrHandler --> LogParser : uses +SysedrHandler *-- ICompressedFile : owns +SysedrHandler *-- IDdadVersionReader : owns +SysedrHandler *-- INvConfig : owns + +@enduml diff --git a/score/datarouter/doc/design/uml/persistent_logging_sequence.puml b/score/datarouter/doc/design/uml/persistent_logging_sequence.puml new file mode 100644 index 00000000..d575f7fb --- /dev/null +++ b/score/datarouter/doc/design/uml/persistent_logging_sequence.puml @@ -0,0 +1,98 @@ +@startuml persistent_logging_sequence +title Persistent Logging Sequence Diagram + +participant "SocketServer" as SS +participant "DataRouter" as DR +participant "DltLogServer" as DLS +participant "SysedrFactory" as SF +participant "SysedrHandler" as SH +participant "LogParser" as LP +participant "DltVerboseHandler" as VH +participant "DltNonverboseHandler" as NH +participant "Client" as CL +participant "ZlibCompressedFile" as ZF + +== System Initialization == +SS -> DR : create() +SS -> DLS : create(staticConfig, enabled) +DLS -> SF : CreateSysedrHandler() +SF -> SH : new SysedrHandler() +SF --> DLS : sysedr_handler_ + +== Handler Registration == +DLS -> LP : add_handlers(parser) +LP -> SH : add_global_handler(*sysedr_handler_) +LP -> SH : add_type_handler(PERSISTENT_REQUEST_TYPE_NAME, *sysedr_handler_) +LP -> NH : add_global_handler(nvhandler_) +LP -> VH : add_type_handler(LOG_ENTRY_TYPE_NAME, vhandler_) + +== Normal Message Processing == +DR -> LP : Parse(SharedMemoryRecord) + +alt Verbose LogEntry + LP -> VH : handle(timestamp, data, size) + VH -> DLS : sendVerbose(tmsp, entry) + LP -> SH : handle(TypeInfo, timestamp, data, size) + SH -> SH : saveVerbose(ecuId, tmsp, entry, write_to_file=false) + SH -> SH : saveDltMsgToBuffer(dlt_message) + note right of SH : Buffer in circular_buffer + +else NonVerbose message + LP -> NH : handle(TypeInfo, timestamp, data, size) + NH -> DLS : sendNonVerbose(desc, tmsp, data, size) + LP -> SH : handle(TypeInfo, timestamp, data, size) + SH -> SH : saveNonVerbose(ecuId, desc, tmsp, data, size) + SH -> SH : saveDltMsgToBuffer(dlt_message) +end + +== Persistent Logging Trigger == +CL -> DR : send PersistentLoggingRequestStructure +note right of CL + Request contains: + - log_file_destination_path + - reason + - triggered_by +end note +DR -> LP : Parse(PersistentLoggingRequestStructure) +LP -> SH : handle(timestamp, request_data, size) +SH -> SH : deserialize(data, size, request) +SH -> SH : selectDestAndOpenFile(log_file_destination_path) +SH -> ZF : open(filename) +SH -> SH : writeContextMessage(request) +SH -> SH : saveBufferToFile() + +loop for each buffered message + SH -> ZF : write(dlt_data, size) +end + +SH -> SH : closeFile() + +== Continuous Persistent Logging == +DR -> LP : Parse(new messages) +LP -> SH : handle(TypeInfo, timestamp, data, size) + +alt persistent logging active + SH -> SH : saveDltMsgToFile(dlt_message) + SH -> ZF : write(dlt_data, size) +else normal mode + SH -> SH : saveDltMsgToBuffer(dlt_message) +end + +note over SH +**Key SysedrHandler Functions:** +- saveDltMsgToBuffer(): Buffer messages normally +- saveBufferToFile(): Flush buffer on trigger +- saveDltMsgToFile(): Direct file writing +- saveVerbose()/saveNonVerbose(): Format messages +- selectDestAndOpenFile(): Open compressed file +- writeContextMessage(): Write request context +- closeFile(): Close compressed file +end note + +note over VH, NH +**Other DLT Handlers:** +Continue processing for +regular DLT output +end note + +@enduml diff --git a/score/datarouter/doc/design/uml/seq-trace.png b/score/datarouter/doc/design/uml/seq-trace.png new file mode 100644 index 00000000..596a3629 Binary files /dev/null and b/score/datarouter/doc/design/uml/seq-trace.png differ diff --git a/score/datarouter/doc/design/uml/seq-trace.puml b/score/datarouter/doc/design/uml/seq-trace.puml new file mode 100644 index 00000000..21dc0a26 --- /dev/null +++ b/score/datarouter/doc/design/uml/seq-trace.puml @@ -0,0 +1,46 @@ +@startuml + +title Tracing operation: write + +participant UserFunction +participant TRACE_MACRO +participant "log_entry::instance()" as log_entry +participant "a : log_entry_allocator" as log_entry_allocator +participant "logger::instance()" as loggerinst +participant "serializer" as serializer + +UserFunction -> TRACE_MACRO ++: TRACE_INFO(TYPE variable) + +group First time trace only +TRACE_MACRO -> log_entry ** : instance() +note left +register type TYPE when +creating the log_entry instance +and get back ID +end note +log_entry -> log_entry ++: log_entry() +log_entry -> loggerinst ++: add_type() +log_entry <-- loggerinst-- : id +log_entry --> TRACE_MACRO --: instance& +end + +||| + +group Normal serialization process +TRACE_MACRO -> log_entry ++: operator=(variable) +log_entry -> log_entry_allocator** : log_entry_allocator() +activate log_entry_allocator + +serializer <- log_entry **: serializer() +activate serializer +serializer -> log_entry_allocator++: allocate() +serializer <-- log_entry_allocator--: buffer +log_entry <-- serializer--: serialized_data +destroy serializer + +TRACE_MACRO <-- log_entry --: success +end + +UserFunction <-- TRACE_MACRO-- + +@enduml diff --git a/score/datarouter/doc/design/uml/seq-tracing-startup.png b/score/datarouter/doc/design/uml/seq-tracing-startup.png new file mode 100644 index 00000000..6e1adf23 Binary files /dev/null and b/score/datarouter/doc/design/uml/seq-tracing-startup.png differ diff --git a/score/datarouter/doc/design/uml/seq-tracing-startup.puml b/score/datarouter/doc/design/uml/seq-tracing-startup.puml new file mode 100644 index 00000000..ec735b94 --- /dev/null +++ b/score/datarouter/doc/design/uml/seq-tracing-startup.puml @@ -0,0 +1,23 @@ +@startuml + +title Tracing startup + +participant main as main +participant TracingLifecycle +participant "logger::instance : Logger" as logger +participant "writer : MwsrWriter" as writer + +main -> TracingLifecycle ++: Start() + +TracingLifecycle -> logger ++: instance() +logger -> logger: Logger() +logger -> writer **: MwsrWriter() +activate writer +logger --> TracingLifecycle --: logger_instance + +TracingLifecycle -> writer ++: initialize() +TracingLifecycle <-- writer --: success + +main <-- TracingLifecycle --: startup_complete + +@enduml diff --git a/score/datarouter/doc/design/uml/seq_kvs_threads.png b/score/datarouter/doc/design/uml/seq_kvs_threads.png new file mode 100644 index 00000000..17c8d8b2 Binary files /dev/null and b/score/datarouter/doc/design/uml/seq_kvs_threads.png differ diff --git a/score/datarouter/doc/design/uml/seq_kvs_threads_sequence_diagram.puml b/score/datarouter/doc/design/uml/seq_kvs_threads_sequence_diagram.puml new file mode 100644 index 00000000..b0f3ade7 --- /dev/null +++ b/score/datarouter/doc/design/uml/seq_kvs_threads_sequence_diagram.puml @@ -0,0 +1,70 @@ +@startuml kvs_threads_sequence_diagram + +participant "DataRouterConfigurator App" as DRConf +participant "DiagnosticManager" as DiagM +participant "DataRouterConfiguratorService" as DRConfService +participant "UnixDomainClient" as Client +participant "UnixDomainServer" as Server +participant "UnixDomainSocket" as Socket +participant "ConfigSession" as ConfigSession +participant "DltLogServer" as DLTS +participant "PersistentDictionary" as PD + +== Runtime Phase: Enable/Disable Event, Write/Read to KVS == +note over Socket + Acts as the transport layer, handling low-level IPC + and forwarding raw data between the client and server. +end note + +-> DRConf: Run +note over DRConf + Client connections are established, and diagnostic jobs are initialized. +end note + +DiagM -> DRConfService: SerializedStart(...) +note over DRConfService + Initializes runtime environment for diagnostic jobs. +end note + +DRConfService -> Client: send_dlt_config_command +note over Client + The server creates a ConfigSession to process + client commands and maintains the session for communication. +end note + +Client -> Server: send_dlt_config_command +Server -> Socket: receive_dlt_config_command +Socket -> Server: forward_dlt_config_command +Server -> ConfigSession: process_dlt_config_command + +ConfigSession -> DLTS: Process command +note over DLTS +- Parse the diagnostic job command. +- Prepare the required response. +- Save or retrieve DLT configuration in persistent storage. +end note + +DLTS -> PD: Save/Retrieve DLT configuration or update dlt_enabled flag +note over PD +- Save configuration using writeDlt(). +- Retrieve configuration using readDlt(). +- Update dlt_enabled using writeDltEnabled(). +end note + +DLTS -> ConfigSession: Return diagnostic job response +ConfigSession -> Socket: forward_diag_job_response +Socket -> Server: forward_diag_job_response +Server -> Client: diag_job_response + +Client -> DRConfService: diag_job_response +DRConfService -> DRConf: diag_job_result + +deactivate DRConf +deactivate DRConfService +deactivate Server +deactivate Socket +deactivate ConfigSession +deactivate DLTS +deactivate PD + +@enduml diff --git a/score/datarouter/doc/design/uml/sequence_early_disconnect_race.puml b/score/datarouter/doc/design/uml/sequence_early_disconnect_race.puml new file mode 100644 index 00000000..015b7b5d --- /dev/null +++ b/score/datarouter/doc/design/uml/sequence_early_disconnect_race.puml @@ -0,0 +1,71 @@ +@startuml sequence_early_disconnect_race + +title Race: Client Disconnects during OnConnectRequest + +skinparam sequenceMessageAlign center +skinparam responseMessageBelowArrow true + +box "Client Process" #LightBlue + participant "Client" as C +end box + +box "Datarouter Process" #LightGreen + participant "Server Engine\n(message thread)" as SE + participant "OnConnectRequest\n(processes connect)" as OC + participant "disconnect_callback\n(connection teardown)" as DC +end box + +== Client sends kConnect, then crashes == + +C -> SE : kConnect(appid, uid, pid) +SE -> OC : MessageCallback → OnConnectRequest() + +note over OC + OnConnectRequest releases mutex + to call session factory (avoids + deadlock with subscriber mutex). +end note + +OC -> OC : Release mutex_\nCall factory_(pid, conn, ...) + +C -[#red]x C : **Client crashes / exits** + +SE -> DC : disconnect_callback(pid) + +note over DC #PeachPuff + **Unsafe**: pid not yet in + pid_session_map_, so disconnect + cannot clear connection_ → stale + pointer stored when OnConnect + resumes. +end note + +DC -> DC : Lock mutex_\npid NOT in pid_session_map_ + +alt Unsafe (control flow) + DC -> DC : pid not found → no-op + DC -> DC : Unlock mutex_ + + OC -> OC : Factory returns session\nLock mutex_ + OC -> OC : Emplace session in map + OC -> OC : **Store &connection**\n(already destroyed!) + note over OC #Tomato + connection_ is a dangling pointer. + Next Notify() call → undefined behavior. + end note + +else Safe (control flow) + DC -> DC : pid not found in map\n→ Insert pid into\n**disconnected_pids_** + DC -> DC : Unlock mutex_ + + OC -> OC : Factory returns session\nLock mutex_ + OC -> OC : Check disconnected_pids_\npid IS present + OC -> OC : **Refuse to store session**\nErase pid from disconnected_pids_ + note over OC #LightGreen + Session is discarded safely. + No dangling connection pointer. + Client can reconnect cleanly. + end note +end + +@enduml diff --git a/score/datarouter/include/daemon/dlt_log_server.h b/score/datarouter/include/daemon/dlt_log_server.h index 598121ab..3c86fe1f 100644 --- a/score/datarouter/include/daemon/dlt_log_server.h +++ b/score/datarouter/include/daemon/dlt_log_server.h @@ -187,6 +187,10 @@ class DltLogServer : score::platform::datarouter::DltNonverboseHandlerType::IOut AssignmentAction assignment_flag) override; std::string SetDltOutputEnable(bool enable) override; + /// Returns the current output-enable state. + /// Thread-safe: reads std::atomic with acquire ordering. + bool IsOutputEnabled() const noexcept override final; + // This is used for test purpose only in google tests, to have an access to the private members. // Do not use this calls in implementation except unit tests. class DltLogServerTest; @@ -311,8 +315,6 @@ class DltLogServer : score::platform::datarouter::DltNonverboseHandlerType::IOut std::unique_ptr sysedr_handler_; - bool IsOutputEnabled() const noexcept override final; - void SendNonVerbose(const score::mw::log::config::NvMsgDescriptor& desc, uint32_t tmsp, const void* data, diff --git a/score/datarouter/include/daemon/i_dlt_log_server.h b/score/datarouter/include/daemon/i_dlt_log_server.h index fdcdd1cb..e9ea3f9f 100644 --- a/score/datarouter/include/daemon/i_dlt_log_server.h +++ b/score/datarouter/include/daemon/i_dlt_log_server.h @@ -60,6 +60,10 @@ class IDltLogServer AssignmentAction assignment_flag) = 0; virtual std::string SetDltOutputEnable(bool enable) = 0; + /// Returns the current output-enable state. + /// Thread-safe: backed by std::atomic in the concrete implementation. + virtual bool IsOutputEnabled() const noexcept = 0; + virtual ~IDltLogServer() = default; }; diff --git a/score/datarouter/include/daemon/message_passing_server.h b/score/datarouter/include/daemon/message_passing_server.h index 6f897541..b95435fd 100644 --- a/score/datarouter/include/daemon/message_passing_server.h +++ b/score/datarouter/include/daemon/message_passing_server.h @@ -18,7 +18,6 @@ #include "score/mw/log/detail/data_router/shared_memory/common.h" -#include "score/message_passing/i_client_factory.h" #include "score/message_passing/i_server_connection.h" #include "score/message_passing/i_server_factory.h" #include "score/mw/log/detail/logging_identifier.h" @@ -28,6 +27,7 @@ #include "score/concurrency/interruptible_wait.h" #include +#include #include #include #include @@ -35,6 +35,7 @@ #include #include #include +#include namespace score { @@ -68,25 +69,37 @@ class IMessagePassingServerSessionWrapper class MessagePassingServer : public IMessagePassingServerSessionWrapper { public: - class SessionHandle : public daemon::ISessionHandle + struct AcquireWatchdogConfig { - public: - SessionHandle(pid_t pid, - MessagePassingServer* server, - score::cpp::pmr::unique_ptr sender) - : daemon::ISessionHandle(), sender_(std::move(sender)), pid_(pid), server_(server), sender_state_{} + AcquireWatchdogConfig(std::chrono::milliseconds deadline_in = std::chrono::milliseconds{1000}, + std::uint32_t max_misses_in = 3U) + : deadline(deadline_in), max_misses(max_misses_in) { } + std::chrono::milliseconds deadline; + std::uint32_t max_misses; + }; + class SessionHandle : public daemon::ISessionHandle + { + public: + SessionHandle(pid_t pid, MessagePassingServer* server) : daemon::ISessionHandle(), pid_(pid), server_(server) {} + bool AcquireRequest() const override; private: - score::cpp::pmr::unique_ptr sender_; pid_t pid_; MessagePassingServer* server_; - mutable std::optional sender_state_; }; + /// Thread-safety contract: + /// - Tick() is called from the worker thread without the server mutex held. + /// - OnAcquireResponse() is called from the dispatch thread with the server mutex held. + /// - OnClosedByPeer() is called from the worker thread without the server mutex held. + /// - IsSourceClosed() is called from the worker thread with the server mutex held. + /// Implementations must ensure that shared state accessed by Tick() and by any method + /// called under the mutex (OnAcquireResponse, IsSourceClosed) is properly synchronized + /// (e.g., via atomics or an internal lock). class ISession { public: @@ -102,31 +115,39 @@ class MessagePassingServer : public IMessagePassingServerSessionWrapper const score::mw::log::detail::ConnectMessageFromClient&, score::cpp::pmr::unique_ptr)>; - explicit MessagePassingServer(SessionFactory factory, - std::shared_ptr server_factory = nullptr, - std::shared_ptr client_factory = nullptr); + MessagePassingServer(SessionFactory factory, + std::shared_ptr server_factory = nullptr, + AcquireWatchdogConfig watchdog_config = AcquireWatchdogConfig{}); ~MessagePassingServer() noexcept; // for unit test only. to keep rest of functions in private class MessagePassingServerForTest; private: - void NotifyAcquireRequestFailed(std::int32_t pid); + friend class SessionHandle; + + bool NotifyAcquireRequest(pid_t pid); - void MessageCallback(const score::cpp::span message, const pid_t pid); - void OnConnectRequest(const score::cpp::span message, const pid_t pid); - void OnAcquireResponse(const score::cpp::span message, const pid_t pid); + void MessageCallback(score::message_passing::IServerConnection& connection, score::cpp::span message); + void OnConnectRequest(score::message_passing::IServerConnection& connection, + const score::cpp::span message, + pid_t pid); + void OnAcquireResponse(score::message_passing::IServerConnection& connection, + const score::cpp::span message, + pid_t pid); using TimestampT = std::chrono::steady_clock::time_point; struct SessionWrapper { - SessionWrapper(IMessagePassingServerSessionWrapper* server_instance, - pid_t process_id, - std::unique_ptr session_instance) - : server(server_instance), - pid(process_id), - session(std::move(session_instance)), + SessionWrapper(IMessagePassingServerSessionWrapper* message_passing_server, + pid_t client_pid, + std::unique_ptr message_passing_session) + : server(message_passing_server), + pid(client_pid), + session(std::move(message_passing_session)), + connection(nullptr), + acquire_in_flight(false), enqueued(false), running(false), to_delete(false), @@ -165,6 +186,11 @@ class MessagePassingServer : public IMessagePassingServerSessionWrapper pid_t pid; std::unique_ptr session; + score::message_passing::IServerConnection* connection; + bool acquire_in_flight; + std::optional acquire_deadline; + std::uint32_t acquire_miss_count{0U}; + bool enqueued; bool running; bool to_delete; @@ -187,13 +213,18 @@ class MessagePassingServer : public IMessagePassingServerSessionWrapper score::cpp::jthread worker_thread_; std::condition_variable worker_cond_; // to wake up worker thread std::unordered_map pid_session_map_; + // Tracks client PIDs that disconnected before a session could be created/emplaced. + // This closes a race where OnConnectRequest creates a session outside mutex_ while + // disconnect_callback may already have run and the connection object may be gone. + std::unordered_set disconnected_pids_; std::queue work_queue_; std::atomic workers_exit_; std::condition_variable server_cond_; // to wake up server thread bool session_finishing_; std::shared_ptr server_factory_; - std::shared_ptr client_factory_; + + AcquireWatchdogConfig watchdog_config_; }; } // namespace internal diff --git a/score/datarouter/mocks/daemon/dlt_log_server_mock.h b/score/datarouter/mocks/daemon/dlt_log_server_mock.h index f03bca53..da509f13 100644 --- a/score/datarouter/mocks/daemon/dlt_log_server_mock.h +++ b/score/datarouter/mocks/daemon/dlt_log_server_mock.h @@ -44,6 +44,7 @@ class DltLogServerMock : public IDltLogServer (score::platform::DltidT, score::platform::DltidT, score::platform::DltidT, AssignmentAction), (override)); MOCK_METHOD(std::string, SetDltOutputEnable, (bool), (override)); + MOCK_METHOD(bool, IsOutputEnabled, (), (const, noexcept, override)); }; } // namespace mock diff --git a/score/datarouter/src/daemon/dlt_log_server.cpp b/score/datarouter/src/daemon/dlt_log_server.cpp index 8f6b1b45..8d3145da 100644 --- a/score/datarouter/src/daemon/dlt_log_server.cpp +++ b/score/datarouter/src/daemon/dlt_log_server.cpp @@ -213,11 +213,13 @@ void DltLogServer::InitLogChannelsDefault(const bool reloading) void DltLogServer::SetOutputEnabled(const bool enabled) { - const bool update = (dlt_output_enabled_.load(std::memory_order_acquire) != enabled); - - if (update) + bool expected = !enabled; + // Atomically flips only if the current value differs — avoids the TOCTOU + // window between load() and store() for the flag itself. + if (dlt_output_enabled_.compare_exchange_strong( + expected, enabled, std::memory_order_acq_rel, std::memory_order_acquire)) { - dlt_output_enabled_.store(enabled, std::memory_order_release); + // Entered only by the thread that won the CAS; callback fires exactly once. if (enabled_callback_) { enabled_callback_(enabled); diff --git a/score/datarouter/src/daemon/message_passing_server.cpp b/score/datarouter/src/daemon/message_passing_server.cpp index 4539bfdc..87fc1603 100644 --- a/score/datarouter/src/daemon/message_passing_server.cpp +++ b/score/datarouter/src/daemon/message_passing_server.cpp @@ -19,6 +19,8 @@ #include "score/memory.hpp" #include +#include +#include #include #include #include @@ -99,7 +101,7 @@ void MessagePassingServer::SessionWrapper::EnqueueTickWhileLocked() // coverity[autosar_cpp14_a3_1_1_violation] MessagePassingServer::MessagePassingServer(MessagePassingServer::SessionFactory factory, std::shared_ptr server_factory, - std::shared_ptr client_factory) + AcquireWatchdogConfig watchdog_config) : IMessagePassingServerSessionWrapper(), factory_{std::move(factory)}, mutex_{}, @@ -112,7 +114,7 @@ MessagePassingServer::MessagePassingServer(MessagePassingServer::SessionFactory server_cond_{}, session_finishing_{false}, server_factory_{server_factory}, - client_factory_{client_factory} + watchdog_config_{watchdog_config} { worker_thread_ = score::cpp::jthread([this]() { RunWorkerThread(); @@ -136,17 +138,25 @@ MessagePassingServer::MessagePassingServer(MessagePassingServer::SessionFactory MessagePassingConfig::kMaxQueuedNotifies}; receiver_ = server_factory_->Create(kServiceProtocolConfig, kServerConfig); - auto connect_callback = [](score::message_passing::IServerConnection& connection) noexcept -> std::uintptr_t { + auto connect_callback = [this_ptr = + this](score::message_passing::IServerConnection& connection) noexcept -> std::uintptr_t { const pid_t client_pid = connection.GetClientIdentity().pid; + { + std::lock_guard lock(this_ptr->mutex_); + this_ptr->disconnected_pids_.erase(client_pid); + } return static_cast(client_pid); }; - auto disconnect_callback = [mutex_ptr = &mutex_, pid_session_map_ptr = &pid_session_map_]( - score::message_passing::IServerConnection& connection) noexcept { - std::unique_lock lock(*mutex_ptr); - const auto found = pid_session_map_ptr->find(connection.GetClientIdentity().pid); - if (found != pid_session_map_ptr->end()) + auto disconnect_callback = [this_ptr = this](score::message_passing::IServerConnection& connection) noexcept { + const pid_t client_pid = connection.GetClientIdentity().pid; + std::unique_lock lock(this_ptr->mutex_); + this_ptr->disconnected_pids_.insert(client_pid); + + const auto found = this_ptr->pid_session_map_.find(client_pid); + if (found != this_ptr->pid_session_map_.end()) { SessionWrapper& wrapper = found->second; + wrapper.connection = nullptr; wrapper.to_force_finish = true; found->second.EnqueueForDeleteWhileLocked(true); } @@ -154,8 +164,7 @@ MessagePassingServer::MessagePassingServer(MessagePassingServer::SessionFactory auto received_send_message_callback = [this_ptr = this]( score::message_passing::IServerConnection& connection, const score::cpp::span message) noexcept -> score::cpp::blank { - const pid_t client_pid = connection.GetClientIdentity().pid; - this_ptr->MessageCallback(message, client_pid); + this_ptr->MessageCallback(connection, message); return {}; }; auto received_send_message_with_reply_callback = @@ -246,6 +255,19 @@ void MessagePassingServer::RunWorkerThread() } else { + auto& wrapper = ps.second; + if (wrapper.acquire_in_flight && wrapper.acquire_deadline.has_value() && + (now >= *wrapper.acquire_deadline)) + { + wrapper.acquire_in_flight = false; + ++wrapper.acquire_miss_count; + wrapper.acquire_deadline.reset(); + if (wrapper.acquire_miss_count >= watchdog_config_.max_misses) + { + wrapper.EnqueueForDeleteWhileLocked(true); + continue; + } + } ps.second.EnqueueTickWhileLocked(); } } @@ -364,8 +386,10 @@ void MessagePassingServer::FinishPreviousSessionWhileLocked( }); } -void MessagePassingServer::MessageCallback(const score::cpp::span message, const pid_t pid) +void MessagePassingServer::MessageCallback(score::message_passing::IServerConnection& connection, + score::cpp::span message) { + const pid_t pid = connection.GetClientIdentity().pid; if (message.empty()) { std::cerr << "MessagePassingServer: Empty message received from " << pid; @@ -377,10 +401,10 @@ void MessagePassingServer::MessageCallback(const score::cpp::span message, const pid_t pid) +void MessagePassingServer::OnConnectRequest(score::message_passing::IServerConnection& connection, + const score::cpp::span message, + const pid_t pid) { score::mw::log::detail::ConnectMessageFromClient conn; @@ -411,42 +437,12 @@ void MessagePassingServer::OnConnectRequest(const score::cpp::span conn_span{static_cast(static_cast(&conn)), sizeof(conn)}; - std::ignore = std::copy(message.begin(), message.end(), conn_span.begin()); + std::ignore = std::copy_n( + message.begin(), std::min(message.size(), static_cast(conn_span.size())), conn_span.begin()); auto appid_sv = conn.GetAppId().GetStringView(); std::string appid{appid_sv.data(), appid_sv.size()}; - // LCOV_EXCL_START: false positive since it is tested. - std::string client_receiver_name; - // LCOV_EXCL_STOP - if (true == conn.GetUseDynamicIdentifier()) - { - /* - this is private functions so it cannot be test. - */ - // LCOV_EXCL_START - std::string random_part; - for (const auto& s : conn.GetRandomPart()) - { - random_part += s; - } - client_receiver_name = std::string("/logging-") + random_part; - // LCOV_EXCL_STOP - } - else - { - client_receiver_name = std::string("/logging.") + appid + "." + std::to_string(conn.GetUid()); - } - - score::cpp::pmr::memory_resource* memory_resource = score::cpp::pmr::get_default_resource(); - - const score::message_passing::ServiceProtocolConfig protocol_config{ - client_receiver_name, MessagePassingConfig::kMaxMessageSize, 0U, 0U}; - constexpr bool kTrulyAsyncSetToTrue = true; - const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, kTrulyAsyncSetToTrue, false}; - - auto sender = client_factory_->Create(protocol_config, client_config); - // check for timeout or exit request if (stop_source_.stop_requested()) { @@ -462,8 +458,9 @@ void MessagePassingServer::OnConnectRequest(const score::cpp::span session_handle{ - ::score::cpp::pmr::make_unique(memory_resource, pid, this, std::move(sender))}; + ::score::cpp::pmr::make_unique(memory_resource, pid, this)}; auto session = factory_(pid, conn, std::move(session_handle)); if (session) { @@ -481,13 +478,33 @@ void MessagePassingServer::OnConnectRequest(const score::cpp::span lock(mutex_); + if (disconnected_pids_.find(pid) != disconnected_pids_.end()) + { + // Client disconnected before we could create/emplace the session. + // Do not store &connection (may be already destroyed by the framework). + return; + } + auto emplace_result = pid_session_map_.emplace(pid, SessionWrapper{this, pid, std::move(session)}); + if (!emplace_result.second) + { + // Existing session for this PID is still present; do not overwrite it. + // The reconnect path is handled by disconnect_callback + worker-thread teardown. + std::cerr << "MessagePassingServer: Session for pid " << pid << " already exists, dropping new session" + << std::endl; + return; + } + + // connection_ points to framework-owned object. It is cleared in disconnect_callback. + emplace_result.first->second.connection = &connection; // enqueue the tick to speed up processing connection emplace_result.first->second.EnqueueTickWhileLocked(); } } -void MessagePassingServer::OnAcquireResponse(const score::cpp::span message, const pid_t pid) +void MessagePassingServer::OnAcquireResponse(score::message_passing::IServerConnection& connection, + const score::cpp::span message, + const pid_t pid) { std::lock_guard lock(mutex_); const auto found = pid_session_map_.find(pid); @@ -495,6 +512,10 @@ void MessagePassingServer::OnAcquireResponse(const score::cpp::span acq_span{static_cast(static_cast(&acq)), sizeof(acq)}; - std::ignore = std::copy(message.begin(), message.end(), acq_span.begin()); + std::ignore = std::copy_n( + message.begin(), std::min(message.size(), static_cast(acq_span.size())), acq_span.begin()); session.session->OnAcquireResponse(acq); + session.acquire_in_flight = false; + session.acquire_deadline.reset(); + session.acquire_miss_count = 0U; // enqueue the tick to speed up processing acquire response session.EnqueueTickWhileLocked(); } } -void MessagePassingServer::NotifyAcquireRequestFailed(std::int32_t pid) +bool MessagePassingServer::NotifyAcquireRequest(const pid_t pid) { + // Guard against calls during server destruction (e.g., from session destructors + // during pid_session_map_.clear()). workers_exit_ is set before the map is cleared. + if (workers_exit_.load()) + { + return false; + } + std::lock_guard lock(mutex_); const auto found = pid_session_map_.find(pid); if (found == pid_session_map_.end()) { - /* - Code will be hit only in case of pid changed, - but since this is private functions so it cannot be test. - */ - // LCOV_EXCL_START - return; - // LCOV_EXCL_STOP + return false; } - found->second.EnqueueForDeleteWhileLocked(true); -} -bool MessagePassingServer::SessionHandle::AcquireRequest() const -{ - if (!sender_state_.has_value()) + auto& wrapper = found->second; + if (wrapper.connection == nullptr) { - sender_->Start(score::message_passing::IClientConnection::StateCallback{}, - score::message_passing::IClientConnection::NotifyCallback{}); + return false; } - sender_state_ = sender_->GetState(); - if (sender_state_ != score::message_passing::IClientConnection::State::kReady) + if (wrapper.acquire_in_flight) { - return false; + return true; } + + // Notify() is non-blocking (QNX pulse), so holding the mutex is safe and avoids + // a use-after-free race where disconnect_callback could destroy the connection + // object between releasing the lock and calling Notify(). constexpr std::array kMessage{score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireRequest)}; - auto ret = sender_->Send(kMessage); + auto ret = wrapper.connection->Notify(kMessage); + if (!ret) { - if (server_ != nullptr) + // ENOBUFS indicates the notify pool is temporarily exhausted (a previous notification is still in flight). + // This is a transient condition — the watchdog will handle it if the client never responds. + if (ret.error().GetOsDependentErrorCode() == ENOBUFS) { - server_->NotifyAcquireRequestFailed(pid_); + std::cerr << "MessagePassingServer: Notify pool exhausted for pid " << pid << ", skipping acquire request" + << std::endl; return true; } + std::cerr << "MessagePassingServer: Notify failed for pid " << pid << ": " << ret.error() << std::endl; + wrapper.EnqueueForDeleteWhileLocked(true); + } + else + { + wrapper.acquire_in_flight = true; + wrapper.acquire_deadline = TimestampT::clock::now() + watchdog_config_.deadline; } return true; } +bool MessagePassingServer::SessionHandle::AcquireRequest() const +{ + if (server_ == nullptr) + { + return false; + } + return server_->NotifyAcquireRequest(pid_); +} + } // namespace internal } // namespace platform } // namespace score diff --git a/score/datarouter/src/daemon/persistentlogging_config.cpp b/score/datarouter/src/daemon/persistentlogging_config.cpp index 5a585c1f..616b3001 100644 --- a/score/datarouter/src/daemon/persistentlogging_config.cpp +++ b/score/datarouter/src/daemon/persistentlogging_config.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -39,11 +40,20 @@ const std::string kDefaultPersistentLoggingJsonFilepath = "etc/persistent-loggin PersistentLoggingConfig ReadPersistentLoggingConfig(const std::string& file_path) { using ReadResult = PersistentLoggingConfig::ReadResult; - using FileCloseFn = int(*)(std::FILE*); PersistentLoggingConfig config; - using UniqueFileT = std::unique_ptr; - UniqueFileT fp(std::fopen(file_path.c_str(), "r"), &fclose); + struct FileCloser + { + void operator()(std::FILE* file) const noexcept + { + if (file != nullptr) + { + std::fclose(file); + } + } + }; + using UniqueFileT = std::unique_ptr; + UniqueFileT fp(std::fopen(file_path.c_str(), "r"), FileCloser{}); if (nullptr == fp) { config.read_result = ReadResult::kErrorOpen; diff --git a/score/datarouter/src/daemon/socketserver.cpp b/score/datarouter/src/daemon/socketserver.cpp index b936338d..ddcba930 100644 --- a/score/datarouter/src/daemon/socketserver.cpp +++ b/score/datarouter/src/daemon/socketserver.cpp @@ -171,6 +171,7 @@ std::unique_ptr SocketServer::Crea { auto global_handlers = dlt_server_.GetGlobalHandlers(); score::platform::internal::LogParser::HandleRequestMap handle_request_map; + for (auto& binding : dlt_server_.GetTypeHandlerBindings()) { handle_request_map.emplace(std::move(binding.type_name), binding.handler); @@ -226,7 +227,7 @@ std::function SocketServer::CreateEnableHandler(DataRouter& router, score::mw::log::LogWarn() << "Changing output enable to " << enable; WriteDltEnabled(enable, persistent_dictionary); router.ForEachSource(enable); - dlt_server.SetDltOutputEnabled(enable); + dlt_server.SetDltOutputEnable(enable); }; } @@ -294,7 +295,7 @@ std::unique_ptr SocketServer::CreateMessagePassi const auto fd = maybe_fd.value(); const auto quota = dlt_server.GetQuota(appid); const auto quota_enforcement_enabled = dlt_server.GetQuotaEnforcementEnabled(); - const bool is_dlt_enabled = dlt_server.GetDltEnabled(); + const bool is_dlt_enabled = dlt_server.IsOutputEnabled(); auto source_session = router.NewSourceSession( fd, appid, is_dlt_enabled, std::move(handle), quota, quota_enforcement_enabled, client_pid, nv_config); // The reason for banning is, because it's error-prone to use. One should use abstractions e.g. provided by @@ -384,8 +385,6 @@ void SocketServer::DoWork(const std::atomic_bool& exit_requested, const bool no_ }; std::shared_ptr server_factory = std::make_shared(); - std::shared_ptr client_factory = - std::make_shared(/*server_factory_->GetEngine() Ticket-234313*/); /* Deviation from Rule A5-1-4: @@ -394,7 +393,7 @@ void SocketServer::DoWork(const std::atomic_bool& exit_requested, const bool no_ - mp_server does not exist inside any lambda. */ // coverity[autosar_cpp14_a5_1_4_violation: FALSE] - MessagePassingServer mp_server(mp_factory, std::move(server_factory), std::move(client_factory)); + MessagePassingServer mp_server(mp_factory, std::move(server_factory)); // Run main event loop RunEventLoop(exit_requested, router, *dlt_server, stats_logger); diff --git a/score/datarouter/test/ut/ut_logging/test_dltprotocol.cpp b/score/datarouter/test/ut/ut_logging/test_dltprotocol.cpp index f004187f..ac006ad5 100644 --- a/score/datarouter/test/ut/ut_logging/test_dltprotocol.cpp +++ b/score/datarouter/test/ut/ut_logging/test_dltprotocol.cpp @@ -90,7 +90,7 @@ TEST(DltProtocolTest, DISABLED_PackageFileDataShallReturnsNulloptIfItWorkedOnAlr ASSERT_TRUE(file != nullptr) << "The file used in the unit test is missed! The file: " << kFileName; // Close the file immediately. fclose(file); - file = nullptr; + file = nullptr; auto result = PackageFileData(data_span, file, serial_number, pkg_number); EXPECT_EQ(result, std::nullopt); diff --git a/score/datarouter/test/ut/ut_logging/test_dltserver.cpp b/score/datarouter/test/ut/ut_logging/test_dltserver.cpp index 7eab7882..5866614f 100644 --- a/score/datarouter/test/ut/ut_logging/test_dltserver.cpp +++ b/score/datarouter/test/ut/ut_logging/test_dltserver.cpp @@ -16,6 +16,10 @@ #include "daemon/dlt_log_server.h" #include "score/datarouter/include/daemon/configurator_commands.h" #include "score/datarouter/mocks/daemon/log_sender_mock.h" +#include +#include +#include +#include #include "gtest/gtest.h" @@ -151,7 +155,7 @@ TEST_F(DltServerCreatedWithoutConfigFixture, WhenCreatedDefault) TEST_F(DltServerCreatedWithoutConfigFixture, WhenCreatedDefaultDltEnabledTrue) { DltLogServer dlt_server(s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), true); - const auto dlt_enabled = dlt_server.GetDltEnabled(); + const auto dlt_enabled = dlt_server.IsOutputEnabled(); EXPECT_TRUE(dlt_enabled); } @@ -1006,7 +1010,7 @@ TEST_F(DltServerCreatedWithoutConfigFixture, SendFTVerboseAppIdNoCoreChannelExpe dlt_server.SendFtVerbose({}, score::mw::log::LogLevel::kVerbose, app_id, ctx_id, 0U, 100U); } -TEST_F(DltServerCreatedWithoutConfigFixture, SetDltOutputEnabledToTrueExpectDltOutputEnabledFlagTrue) +TEST_F(DltServerCreatedWithoutConfigFixture, SetDltOutputEnableToTrueExpectIsOutputEnabledTrue) { EXPECT_CALL(read_callback, Call()).Times(0); EXPECT_CALL(write_callback, Call(_)).Times(0); @@ -1014,8 +1018,8 @@ TEST_F(DltServerCreatedWithoutConfigFixture, SetDltOutputEnabledToTrueExpectDltO score::logging::dltserver::DltLogServer::DltLogServerTest dlt_server( s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), true); - dlt_server.SetDltOutputEnabled(true); - EXPECT_TRUE(dlt_server.GetDltEnabled()); + dlt_server.SetDltOutputEnable(true); + EXPECT_TRUE(dlt_server.IsOutputEnabled()); } TEST_F(DltServerCreatedWithConfigFixture, SetLogChannelThresholdChannelMissingDirectCallReturnsError) @@ -1150,20 +1154,20 @@ TEST_F(DltServerCreatedWithConfigFixture, SetDltOutputEnableDirectCall) auto response = dlt_server.SetDltOutputEnable(true); EXPECT_EQ(response.size(), kCommandResponseSize); EXPECT_EQ(response[0], static_cast(config::kRetOk)); - EXPECT_TRUE(dlt_server.GetDltEnabled()); + EXPECT_TRUE(dlt_server.IsOutputEnabled()); // Test disabling output through public method response = dlt_server.SetDltOutputEnable(false); EXPECT_EQ(response.size(), kCommandResponseSize); EXPECT_EQ(response[0], static_cast(config::kRetOk)); - EXPECT_FALSE(dlt_server.GetDltEnabled()); + EXPECT_FALSE(dlt_server.IsOutputEnabled()); } TEST_F(DltServerCreatedWithConfigFixture, SetDltOutputEnableBehaviorBlocksAllSends) { // Prove that enabling/disabling output affects the observable server state. // Note: sendVerbose()/sendNonVerbose() are not gated by this flag in the current implementation; - // the flag controls the DLT output enable state exposed via GetDltEnabled(). + // the flag controls the DLT output enable state exposed via IsOutputEnabled(). EXPECT_CALL(read_callback, Call()).Times(1).WillOnce(Return(p_config)); EXPECT_CALL(write_callback, Call(_)).Times(0); @@ -1179,13 +1183,13 @@ TEST_F(DltServerCreatedWithConfigFixture, SetDltOutputEnableBehaviorBlocksAllSen const auto disable_resp = dlt_server.SetDltOutputEnable(false); EXPECT_EQ(disable_resp.size(), kCommandResponseSize); EXPECT_EQ(disable_resp[0], static_cast(config::kRetOk)); - EXPECT_FALSE(dlt_server.GetDltEnabled()); + EXPECT_FALSE(dlt_server.IsOutputEnabled()); // Re-enable output: sending should resume. const auto enable_resp = dlt_server.SetDltOutputEnable(true); EXPECT_EQ(enable_resp.size(), kCommandResponseSize); EXPECT_EQ(enable_resp[0], static_cast(config::kRetOk)); - EXPECT_TRUE(dlt_server.GetDltEnabled()); + EXPECT_TRUE(dlt_server.IsOutputEnabled()); // Basic sanity: calling sendVerbose still forwards to the log sender (2 channels). EXPECT_CALL(*log_sender_mock_raw_ptr, SendVerbose(_, _, _)).Times(2); @@ -1254,4 +1258,188 @@ TEST_F(DltServerCreatedWithConfigFixture, ReadLogChannelNamesDirectCallContainsE EXPECT_NE(response_str.find("CORE"), std::string::npos) << "Response should contain CORE channel"; } +TEST_F(DltServerCreatedWithoutConfigFixture, ConcurrentEnableSameValueNoCallbackFired) +{ + EXPECT_CALL(read_callback, Call()).Times(0); + EXPECT_CALL(write_callback, Call(_)).Times(0); + + DltLogServer::DltLogServerTest server( + s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), /*enabled=*/true); + + std::atomic callback_count{0}; + server.SetEnabledCallback([&](bool /*enabled*/) { + callback_count.fetch_add(1, std::memory_order_relaxed); + }); + + std::thread t1([&] { + server.SetDltOutputEnable(true); + }); + std::thread t2([&] { + server.SetDltOutputEnable(true); + }); + t1.join(); + t2.join(); + + EXPECT_TRUE(server.IsOutputEnabled()); + EXPECT_EQ(callback_count.load(), 0) << "No callback expected: both threads attempted a redundant enable"; +} + +TEST_F(DltServerCreatedWithoutConfigFixture, ConcurrentDisableSameValueCallbackFiredOnce) +{ + EXPECT_CALL(read_callback, Call()).Times(0); + EXPECT_CALL(write_callback, Call(_)).Times(0); + + DltLogServer::DltLogServerTest server( + s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), /*enabled=*/true); + + std::atomic callback_count{0}; + server.SetEnabledCallback([&](bool /*enabled*/) { + callback_count.fetch_add(1, std::memory_order_relaxed); + }); + + std::thread t1([&] { + server.SetDltOutputEnable(false); + }); + std::thread t2([&] { + server.SetDltOutputEnable(false); + }); + t1.join(); + t2.join(); + + EXPECT_FALSE(server.IsOutputEnabled()); + EXPECT_EQ(callback_count.load(), 1) << "Exactly one callback expected: only one thread wins the true->false CAS"; +} + +TEST_F(DltServerCreatedWithoutConfigFixture, ConcurrentOppositeValuesCallbackCountOneOrTwo) +{ + EXPECT_CALL(read_callback, Call()).Times(0); + EXPECT_CALL(write_callback, Call(_)).Times(0); + + DltLogServer::DltLogServerTest server( + s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), /*enabled=*/true); + + std::atomic callback_count{0}; + server.SetEnabledCallback([&](bool /*enabled*/) { + callback_count.fetch_add(1, std::memory_order_relaxed); + }); + + std::thread t_disable([&] { + server.SetDltOutputEnable(false); + }); + std::thread t_enable([&] { + server.SetDltOutputEnable(true); + }); + t_disable.join(); + t_enable.join(); + + const int count = callback_count.load(); + EXPECT_GE(count, 1) << "At least one genuine transition must have fired"; + EXPECT_LE(count, 2) << "At most two genuine transitions are possible"; + + // Flag must be consistent with whichever CAS won last (no corruption). + const bool flag = server.IsOutputEnabled(); + EXPECT_TRUE(flag == true || flag == false); +} + +TEST_F(DltServerCreatedWithoutConfigFixture, ConcurrentToggleStormFlagRemainsConsistent) +{ + EXPECT_CALL(read_callback, Call()).Times(0); + EXPECT_CALL(write_callback, Call(_)).Times(0); + + DltLogServer::DltLogServerTest server( + s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), /*enabled=*/true); + + std::atomic callback_count{0}; + server.SetEnabledCallback([&](bool /*enabled*/) { + callback_count.fetch_add(1, std::memory_order_relaxed); + }); + + constexpr int kThreads = 8; + constexpr int kTogglesEach = 100; + + std::vector threads; + threads.reserve(kThreads); + for (int i = 0; i < kThreads; ++i) + { + const bool start_with_enable = (i % 2 == 0); + threads.emplace_back([&server, start_with_enable]() { + for (int j = 0; j < kTogglesEach; ++j) + { + server.SetDltOutputEnable((j % 2 == 0) ? start_with_enable : !start_with_enable); + } + }); + } + for (auto& t : threads) + { + t.join(); + } + + // IsOutputEnabled() returns std::atomic::load(), so no torn read + // is possible; we only assert the callback bound. + static_cast(server.IsOutputEnabled()); + EXPECT_LE(callback_count.load(), kThreads * kTogglesEach); +} + +TEST_F(DltServerCreatedWithoutConfigFixture, CallbackReceivesCorrectBooleanValuePerTransition) +{ + EXPECT_CALL(read_callback, Call()).Times(0); + EXPECT_CALL(write_callback, Call(_)).Times(0); + + DltLogServer::DltLogServerTest server( + s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), /*enabled=*/true); + + std::vector observed_values; + std::mutex values_mutex; + server.SetEnabledCallback([&](bool enabled) { + std::lock_guard lock(values_mutex); + observed_values.push_back(enabled); + }); + + server.SetDltOutputEnable(false); // genuine: true→false + server.SetDltOutputEnable(false); // redundant: no callback + server.SetDltOutputEnable(true); // genuine: false→true + server.SetDltOutputEnable(true); // redundant: no callback + + ASSERT_EQ(observed_values.size(), 2U); + EXPECT_FALSE(observed_values[0]) << "First transition was true->false"; + EXPECT_TRUE(observed_values[1]) << "Second transition was false->true"; +} + +TEST_F(DltServerCreatedWithoutConfigFixture, ConcurrentReaderSeesOnlyValidBooleanValues) +{ + EXPECT_CALL(read_callback, Call()).Times(0); + EXPECT_CALL(write_callback, Call(_)).Times(0); + + DltLogServer::DltLogServerTest server( + s_config, read_callback.AsStdFunction(), write_callback.AsStdFunction(), /*enabled=*/true); + + std::atomic stop_reader{false}; + + std::thread reader([&] { + // static_assert documents that bool can only be true or false; + // the acquire-load is still exercised so TSan can observe ordering. + static_assert(sizeof(bool) == 1U, "bool must be exactly one byte"); + while (!stop_reader.load(std::memory_order_relaxed)) + { + // Load with acquire semantics to pair with the release-store in + // SetOutputEnabled; the result is intentionally unused here — + // the goal is to exercise the memory-ordering path under TSan. + static_cast(server.IsOutputEnabled()); + } + }); + + std::thread writer([&] { + for (int i = 0; i < 500; ++i) + { + server.SetDltOutputEnable(i % 2 == 0); + } + }); + + writer.join(); + stop_reader.store(true, std::memory_order_relaxed); + reader.join(); + // If TSan is enabled it will have reported any acquire/release violation + // during the run; no additional runtime assertion is needed here. +} + } // namespace test diff --git a/score/datarouter/test/ut/ut_logging/test_message_passing_server.cpp b/score/datarouter/test/ut/ut_logging/test_message_passing_server.cpp index da8447dc..10b66d11 100644 --- a/score/datarouter/test/ut/ut_logging/test_message_passing_server.cpp +++ b/score/datarouter/test/ut/ut_logging/test_message_passing_server.cpp @@ -14,7 +14,6 @@ #include "score/datarouter/include/daemon/message_passing_server.h" #include "score/message_passing/mock/client_connection_mock.h" -#include "score/message_passing/mock/client_factory_mock.h" #include "score/message_passing/mock/server_connection_mock.h" #include "score/message_passing/mock/server_factory_mock.h" #include "score/message_passing/mock/server_mock.h" @@ -28,6 +27,7 @@ #include #include +#include #include #include @@ -79,8 +79,9 @@ constexpr pid_t kClienT0Pid = 1000; constexpr pid_t kClienT1Pid = 1001; constexpr pid_t kClienT2Pid = 1002; constexpr std::uint32_t kMaxSendBytes{17U}; +constexpr std::uint32_t kMaxNotifyBytes{1U}; -std::uint32_t gKReceiverQueueMaxSize = 0; +std::uint32_t gKReceiverQueueMaxSize = 1; class MockSession : public MessagePassingServer::ISession { @@ -153,18 +154,17 @@ class MessagePassingServerFixture : public ::testing::Test void SetUp() override { server_factory_mock = std::make_shared>(); - client_factory_mock = std::make_shared>(); - const score::message_passing::IServerFactory::ServerConfig server_config{gKReceiverQueueMaxSize, 0U, 0U}; + const score::message_passing::IServerFactory::ServerConfig server_config{gKReceiverQueueMaxSize, 0U, 1U}; auto server_ptr = score::cpp::pmr::make_unique>( score::cpp::pmr::get_default_resource()); server_mock = server_ptr.get(); - EXPECT_CALL( - *server_factory_mock, - Create(CompareServiceProtocol(ServiceProtocolConfig{"/logging.datarouter_recv", kMaxSendBytes, 0U, 0U}), - CompareServerConfig(server_config))) + EXPECT_CALL(*server_factory_mock, + Create(CompareServiceProtocol( + ServiceProtocolConfig{"/logging.datarouter_recv", kMaxSendBytes, 0U, kMaxNotifyBytes}), + CompareServerConfig(server_config))) .WillOnce(Return(ByMove(std::move(server_ptr)))); } @@ -212,11 +212,6 @@ class MessagePassingServerFixture : public ::testing::Test return session; }; } - void ExpectClientDestruction(StrictMock<::score::message_passing::ClientConnectionMock>* client_mock) - { - EXPECT_CALL(*client_mock, Destruct()).Times(AnyNumber()); - } - void ExpectServerDestruction() const { EXPECT_CALL(*server_mock, Destruct()).Times(AnyNumber()); @@ -254,7 +249,29 @@ class MessagePassingServerFixture : public ::testing::Test }); // instantiate MessagePassingServer - server.emplace(factory, server_factory_mock, client_factory_mock); + server.emplace(factory, server_factory_mock); + } + + void InstantiateServerWithWatchdog(MessagePassingServer::SessionFactory factory, + MessagePassingServer::AcquireWatchdogConfig watchdog_config) + { + EXPECT_CALL(*server_mock, + StartListening(Matcher(_), + Matcher(_), + Matcher(_), + Matcher(_))) + .WillOnce([this](score::message_passing::ConnectCallback con_callback, + score::message_passing::DisconnectCallback discon_callback, + score::message_passing::MessageCallback sn_callback, + score::message_passing::MessageCallback sn_rep_callback) { + this->connect_callback = std::move(con_callback); + this->disconnect_callback = std::move(discon_callback); + this->sent_callback = std::move(sn_callback); + this->sent_with_reply_callback = std::move(sn_rep_callback); + return score::cpp::expected_blank{}; + }); + + server.emplace(factory, server_factory_mock, watchdog_config); } auto CreateConnectMessageSample(const pid_t) @@ -271,27 +288,25 @@ class MessagePassingServerFixture : public ::testing::Test return message; } - StrictMock<::score::message_passing::ClientConnectionMock>* ExpectConnectCallBackCalledAndClientCreated( - const pid_t pid) + StrictMock<::score::message_passing::ServerConnectionMock>* ConnectClientAndSendConnectMessage(const pid_t pid) { - auto client = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - - auto* client_mock = client.get(); + auto connection = std::make_unique>(); + auto* connection_ptr = connection.get(); + auto emplace_result = connections.emplace(pid, std::move(connection)); + EXPECT_TRUE(emplace_result.second); - EXPECT_CALL(*client_factory_mock, - Create(Matcher(_), - Matcher(_))) - .WillOnce(Return(ByMove(std::move(client)))); + connection_identities.insert_or_assign(pid, score::message_passing::ClientIdentity{pid, 0, 0}); + EXPECT_CALL(*connection_ptr, GetClientIdentity()) + .Times(AnyNumber()) + .WillRepeatedly(ReturnRef(connection_identities.at(pid))); - StrictMock<::score::message_passing::ServerConnectionMock> connection; - score::message_passing::ClientIdentity client_identity{pid, 0, 0}; - EXPECT_CALL(connection, GetClientIdentity()).Times(AnyNumber()).WillRepeatedly(ReturnRef(client_identity)); + // Mirror production behavior: connect_callback runs before any messages. + std::ignore = connect_callback(*connection_ptr); auto message = CreateConnectMessageSample(pid); - sent_callback(connection, message); + sent_callback(*connection_ptr, message); - return client_mock; + return connection_ptr; } void UninstantiateServer() @@ -304,11 +319,11 @@ class MessagePassingServerFixture : public ::testing::Test EXPECT_CALL(*unistd_mock, getpid()).WillRepeatedly(Return(kOurPid)); } - void ExpectMessageSendInSequence(const DatarouterMessageIdentifier& id, - ::testing::Sequence& seq, - StrictMock<::score::message_passing::ClientConnectionMock>* client_mock) + void ExpectAcquireNotifyInSequence(const DatarouterMessageIdentifier& id, + ::testing::Sequence& seq, + StrictMock<::score::message_passing::ServerConnectionMock>* connection_mock) { - EXPECT_CALL(*client_mock, Send(An>())) + EXPECT_CALL(*connection_mock, Notify(An>())) .InSequence(seq) .WillOnce([id](const auto m) { score::cpp::expected_blank ret{}; @@ -320,14 +335,13 @@ class MessagePassingServerFixture : public ::testing::Test }); } - void ExpectAndFailShortMessageSend(StrictMock<::score::message_passing::ClientConnectionMock>* client_mock) + void ExpectAndFailAcquireNotify(StrictMock<::score::message_passing::ServerConnectionMock>* connection_mock) { - EXPECT_CALL(*client_mock, Send(Matcher>(_))) + EXPECT_CALL(*connection_mock, Notify(Matcher>(_))) .WillOnce(Return(score::cpp::make_unexpected(score::os::Error::createFromErrno(EINVAL)))); } StrictMock<::score::message_passing::ServerMock>* server_mock{}; - std::shared_ptr> client_factory_mock; std::shared_ptr> server_factory_mock; ::score::os::MockGuard unistd_mock{}; @@ -337,6 +351,9 @@ class MessagePassingServerFixture : public ::testing::Test score::message_passing::MessageCallback sent_callback; score::message_passing::MessageCallback sent_with_reply_callback; + std::unordered_map>> connections; + std::map connection_identities; + std::mutex map_mutex; std::condition_variable map_cond; // currently only used for destruction std::unordered_map session_map; @@ -395,7 +412,7 @@ TEST_F(MessagePassingServerFixture, TestFailedStartListening) return score::cpp::expected_blank{}; }); // instantiate MessagePassingServer - server.emplace(factory, server_factory_mock, client_factory_mock); + server.emplace(factory, server_factory_mock); ExpectServerDestruction(); UninstantiateServer(); @@ -413,7 +430,7 @@ TEST_F(MessagePassingServerFixture, TestStartListeningFailure) Matcher(_))) .WillOnce(Return(score::cpp::make_unexpected(score::os::Error::createFromErrno(EINVAL)))); - server.emplace(factory, server_factory_mock, client_factory_mock); + server.emplace(factory, server_factory_mock); ExpectServerDestruction(); UninstantiateServer(); } @@ -427,40 +444,37 @@ TEST_F(MessagePassingServerFixture, TestOneConnectAcquireRelease) EXPECT_EQ(tick_count, 0); EXPECT_EQ(construct_count, 0); - auto* client = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); + auto* connection_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); EXPECT_EQ(construct_count, 1); - EXPECT_CALL(*client, - Start(Matcher(_), - Matcher(_))); - - EXPECT_CALL(*client, GetState()).WillRepeatedly(Return(score::message_passing::IClientConnection::State::kReady)); - ::testing::Sequence seq; - ExpectMessageSendInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, client); + ExpectAcquireNotifyInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, connection_ptr); + session_map.at(kClienT0Pid).handle->AcquireRequest(); session_map.at(kClienT0Pid).handle->AcquireRequest(); EXPECT_EQ(acquire_response_count, 0); - StrictMock<::score::message_passing::ServerConnectionMock> connection; - score::message_passing::ClientIdentity client_identity{kClienT0Pid, 0, 0}; - EXPECT_CALL(connection, GetClientIdentity()).Times(AnyNumber()).WillRepeatedly(ReturnRef(client_identity)); - score::mw::log::detail::ReadAcquireResult acquire_result{0U}; std::array message{}; message[0] = score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireResponse); std::memcpy(&message[1], &acquire_result, sizeof(acquire_result)); - sent_callback(connection, message); + sent_callback(*connection_ptr, message); EXPECT_EQ(acquire_response_count, 1); + { + std::array bad_message{}; + bad_message[0] = score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireResponse); + sent_callback(*connection_ptr, bad_message); + EXPECT_EQ(acquire_response_count, 2); + } + EXPECT_EQ(closed_by_peer_count, 0); EXPECT_FALSE(session_map.empty()); - ExpectAndFailShortMessageSend(client); + ExpectAndFailAcquireNotify(connection_ptr); ExpectServerDestruction(); - ExpectClientDestruction(client); session_map.at(kClienT0Pid).handle->AcquireRequest(); { // let the worker thread process the fault; wait until it erases the client @@ -484,15 +498,15 @@ TEST_F(MessagePassingServerFixture, TestTripleConnectDifferentPids) EXPECT_EQ(construct_count, 0); - auto* client0 = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); - auto* client1 = ExpectConnectCallBackCalledAndClientCreated(kClienT1Pid); - auto* client2 = ExpectConnectCallBackCalledAndClientCreated(kClienT2Pid); + auto* connection0 = ConnectClientAndSendConnectMessage(kClienT0Pid); + auto* connection1 = ConnectClientAndSendConnectMessage(kClienT1Pid); + auto* connection2 = ConnectClientAndSendConnectMessage(kClienT2Pid); EXPECT_EQ(construct_count, 3); ExpectServerDestruction(); - ExpectClientDestruction(client0); - ExpectClientDestruction(client1); - ExpectClientDestruction(client2); + std::ignore = connection0; + std::ignore = connection1; + std::ignore = connection2; EXPECT_EQ(closed_by_peer_count, 0); EXPECT_EQ(destruct_count, 0); @@ -515,22 +529,34 @@ TEST_F(MessagePassingServerFixture, TestTripleConnectSamePid) EXPECT_EQ(construct_count, 0); // Recieving new connect with old pid means that old pid owner died and disconnect_callback was called. - auto* client0 = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); + auto* connection0 = ConnectClientAndSendConnectMessage(kClienT0Pid); EXPECT_CALL(connection, GetClientIdentity()).WillOnce(ReturnRef(client_identity)); - ExpectClientDestruction(client0); this->disconnect_callback(connection); - using namespace std::chrono_literals; - std::this_thread::sleep_for(100ms); - auto* client1 = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.find(kClienT0Pid) == session_map.end(); + })) << "Timed out waiting for session cleanup after first disconnect"; + } + connections.erase(kClienT0Pid); + auto* connection1 = ConnectClientAndSendConnectMessage(kClienT0Pid); EXPECT_CALL(connection, GetClientIdentity()).WillOnce(ReturnRef(client_identity)); - ExpectClientDestruction(client1); this->disconnect_callback(connection); - std::this_thread::sleep_for(100ms); + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.find(kClienT0Pid) == session_map.end(); + })) << "Timed out waiting for session cleanup after second disconnect"; + } - auto* client2 = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); - ExpectClientDestruction(client2); + connections.erase(kClienT0Pid); + auto* connection2 = ConnectClientAndSendConnectMessage(kClienT0Pid); EXPECT_EQ(construct_count, 3); + std::ignore = connection0; + std::ignore = connection1; + std::ignore = connection2; + ExpectServerDestruction(); EXPECT_EQ(closed_by_peer_count, 2); @@ -543,6 +569,48 @@ TEST_F(MessagePassingServerFixture, TestTripleConnectSamePid) EXPECT_EQ(destruct_count, 3); } +TEST_F(MessagePassingServerFixture, StaleConnectionAcquireResponseShouldBeIgnored) +{ + ExpectOurPidIsQueried(); + InstantiateServer(GetCountingSessionFactory()); + + // Connect client (connection0) and create session. + auto* connection0_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + + // Disconnect the client and wait until its session is torn down. + this->disconnect_callback(*connection0_ptr); + { + std::unique_lock lock(map_mutex); + map_cond.wait(lock, [this]() { + return session_map.empty(); + }); + } + + // Keep the old connection object alive, but free the PID slot for the new connection. + auto old_connection = std::move(connections.at(kClienT0Pid)); + connections.erase(kClienT0Pid); + auto* stale_connection_ptr = old_connection.get(); + + // Reconnect client with the same PID (connection1) and create a new session. + auto* connection1_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + + score::mw::log::detail::ReadAcquireResult acquire_result{0U}; + std::array response{}; + response[0] = score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireResponse); + std::memcpy(&response[1], &acquire_result, sizeof(acquire_result)); + + // Response on stale connection must be ignored. + sent_callback(*stale_connection_ptr, response); + EXPECT_EQ(acquire_response_count, 0); + + // Response on the current connection must be accepted. + sent_callback(*connection1_ptr, response); + EXPECT_EQ(acquire_response_count, 1); + + ExpectServerDestruction(); + UninstantiateServer(); +} + TEST_F(MessagePassingServerFixture, TestSamePidWhileRunning) { @@ -553,32 +621,35 @@ TEST_F(MessagePassingServerFixture, TestSamePidWhileRunning) tick_blocker = true; EXPECT_EQ(tick_count, 0); EXPECT_EQ(construct_count, 0); - auto* client0 = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); - auto* client1 = ExpectConnectCallBackCalledAndClientCreated(kClienT1Pid); - auto* client2 = ExpectConnectCallBackCalledAndClientCreated(kClienT2Pid); + auto* connection0 = ConnectClientAndSendConnectMessage(kClienT0Pid); + auto* connection1 = ConnectClientAndSendConnectMessage(kClienT1Pid); + auto* connection2 = ConnectClientAndSendConnectMessage(kClienT2Pid); EXPECT_EQ(construct_count, 3); ExpectServerDestruction(); - // ExpectClientDestruction(client0); // wait until CLIENT0 is blocked inside the first tick session_map.at(kClienT0Pid).WaitStartOfFirstTick(); - // accumulate other ticks in the queue - using namespace std::chrono_literals; - std::this_thread::sleep_for(100ms); - // we will need to unblock the tick before the callback returns, so start it on a separate thread std::thread connect_thread([&]() { StrictMock<::score::message_passing::ServerConnectionMock> connection; score::message_passing::ClientIdentity client_identity{kClienT0Pid, 0, 0}; EXPECT_CALL(connection, GetClientIdentity()).WillOnce(ReturnRef(client_identity)); - ExpectClientDestruction(client0); this->disconnect_callback(connection); - std::this_thread::sleep_for(100ms); - auto* new_client = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); - ExpectClientDestruction(new_client); + // Wait for the old session to be fully destroyed before reconnecting + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.find(kClienT0Pid) == session_map.end(); + })) << "Timed out waiting for old CLIENT0 session destruction"; + } + + connections.erase(kClienT0Pid); + + auto* new_connection = ConnectClientAndSendConnectMessage(kClienT0Pid); + std::ignore = new_connection; }); EXPECT_EQ(destruct_count, 0); // no destruction while we are still in the tick @@ -591,8 +662,9 @@ TEST_F(MessagePassingServerFixture, TestSamePidWhileRunning) EXPECT_EQ(destruct_count, 1); EXPECT_GE(tick_count, 2); - ExpectClientDestruction(client1); - ExpectClientDestruction(client2); + std::ignore = connection0; + std::ignore = connection1; + std::ignore = connection2; UninstantiateServer(); EXPECT_EQ(closed_by_peer_count, 1); @@ -608,9 +680,9 @@ TEST_F(MessagePassingServerFixture, TestSamePidWhileQueued) tick_blocker = true; EXPECT_EQ(tick_count, 0); EXPECT_EQ(construct_count, 0); - auto* client0 = ExpectConnectCallBackCalledAndClientCreated(kClienT0Pid); - auto* client1 = ExpectConnectCallBackCalledAndClientCreated(kClienT1Pid); - auto* client2 = ExpectConnectCallBackCalledAndClientCreated(kClienT2Pid); + auto* connection0 = ConnectClientAndSendConnectMessage(kClienT0Pid); + auto* connection1 = ConnectClientAndSendConnectMessage(kClienT1Pid); + auto* connection2 = ConnectClientAndSendConnectMessage(kClienT2Pid); EXPECT_EQ(construct_count, 3); ExpectServerDestruction(); @@ -618,21 +690,25 @@ TEST_F(MessagePassingServerFixture, TestSamePidWhileQueued) // wait until CLIENT0 is blocked inside the first tick session_map.at(kClienT0Pid).WaitStartOfFirstTick(); - // accumulate other ticks (CLIENT2 in particular) in the queue - using namespace std::chrono_literals; - std::this_thread::sleep_for(250ms); - // we will need to unblock the tick before the callback returns, so start it on a separate thread std::thread connect_thread([&]() { StrictMock<::score::message_passing::ServerConnectionMock> connection; score::message_passing::ClientIdentity client_identity{kClienT2Pid, 0, 0}; EXPECT_CALL(connection, GetClientIdentity()).WillOnce(ReturnRef(client_identity)); - ExpectClientDestruction(client2); this->disconnect_callback(connection); - std::this_thread::sleep_for(100ms); - auto* new_client = ExpectConnectCallBackCalledAndClientCreated(kClienT2Pid); - ExpectClientDestruction(new_client); + // Wait for the old session to be fully destroyed before reconnecting + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.find(kClienT2Pid) == session_map.end(); + })) << "Timed out waiting for old CLIENT2 session destruction"; + } + + connections.erase(kClienT2Pid); + + auto* new_connection = ConnectClientAndSendConnectMessage(kClienT2Pid); + std::ignore = new_connection; }); EXPECT_EQ(destruct_count, 0); // no destruction while we are still in the tick @@ -645,14 +721,177 @@ TEST_F(MessagePassingServerFixture, TestSamePidWhileQueued) EXPECT_EQ(destruct_count, 1); EXPECT_GE(tick_count, 2); - ExpectClientDestruction(client0); - ExpectClientDestruction(client1); + std::ignore = connection0; + std::ignore = connection1; + std::ignore = connection2; UninstantiateServer(); EXPECT_EQ(closed_by_peer_count, 1); EXPECT_EQ(destruct_count, 4); } +TEST_F(MessagePassingServerFixture, WatchdogShouldTearDownUnresponsiveClient) +{ + ExpectOurPidIsQueried(); + + MessagePassingServer::AcquireWatchdogConfig watchdog_config; + watchdog_config.deadline = std::chrono::milliseconds{0}; + watchdog_config.max_misses = 1U; + InstantiateServerWithWatchdog(GetCountingSessionFactory(), watchdog_config); + + auto* connection_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + ::testing::Sequence seq; + ExpectAcquireNotifyInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, connection_ptr); + session_map.at(kClienT0Pid).handle->AcquireRequest(); + + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.empty(); + })) << "Timed out waiting for watchdog teardown"; + } + + ExpectServerDestruction(); + UninstantiateServer(); +} + +TEST_F(MessagePassingServerFixture, WatchdogMissCountShouldResetOnValidResponse) +{ + ExpectOurPidIsQueried(); + + MessagePassingServer::AcquireWatchdogConfig watchdog_config; + watchdog_config.deadline = std::chrono::milliseconds{0}; + watchdog_config.max_misses = 2U; + InstantiateServerWithWatchdog(GetCountingSessionFactory(), watchdog_config); + + auto* connection_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + + ::testing::Sequence seq; + ExpectAcquireNotifyInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, connection_ptr); + ExpectAcquireNotifyInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, connection_ptr); + ExpectAcquireNotifyInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, connection_ptr); + + // 1) First acquire request is missed by the client -> miss_count becomes 1. + session_map.at(kClienT0Pid).handle->AcquireRequest(); + using namespace std::chrono_literals; + std::this_thread::sleep_for(100ms); + + // 2) Late valid response arrives -> miss_count should reset to 0. + score::mw::log::detail::ReadAcquireResult acquire_result{0U}; + std::array response{}; + response[0] = score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireResponse); + std::memcpy(&response[1], &acquire_result, sizeof(acquire_result)); + sent_callback(*connection_ptr, response); + + // 3) One more missed acquire should NOT tear down (max_misses = 2, miss_count should be 1). + session_map.at(kClienT0Pid).handle->AcquireRequest(); + std::this_thread::sleep_for(100ms); + EXPECT_FALSE(session_map.empty()); + + // 4) Second miss after the reset should now tear down. + session_map.at(kClienT0Pid).handle->AcquireRequest(); + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.empty(); + })) << "Timed out waiting for watchdog teardown after miss-count reset"; + } + + ExpectServerDestruction(); + UninstantiateServer(); +} + +TEST_F(MessagePassingServerFixture, EnobufsFromNotifyShouldNotKillSession) +{ + ExpectOurPidIsQueried(); + + InstantiateServer(GetCountingSessionFactory()); + + auto* connection_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + + // First Notify() fails with ENOBUFS (notify pool exhausted — transient). + EXPECT_CALL(*connection_ptr, Notify(Matcher>(_))) + .WillOnce(Return(score::cpp::make_unexpected(score::os::Error::createFromErrno(ENOBUFS)))); + + session_map.at(kClienT0Pid).handle->AcquireRequest(); + + // Session should still be alive — ENOBUFS is treated as a transient error. + using namespace std::chrono_literals; + std::this_thread::sleep_for(50ms); + EXPECT_FALSE(session_map.empty()); + + // A subsequent successful Notify() should work normally. + ::testing::Sequence seq; + ExpectAcquireNotifyInSequence(DatarouterMessageIdentifier::kAcquireRequest, seq, connection_ptr); + session_map.at(kClienT0Pid).handle->AcquireRequest(); + + // Verify session is still alive after successful notify. + std::this_thread::sleep_for(50ms); + EXPECT_FALSE(session_map.empty()); + + ExpectServerDestruction(); + UninstantiateServer(); +} + +TEST_F(MessagePassingServerFixture, NonEnobufsNotifyFailureShouldStillKillSession) +{ + ExpectOurPidIsQueried(); + + InstantiateServer(GetCountingSessionFactory()); + + auto* connection_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + + // Notify() fails with EINVAL (not ENOBUFS) — should still tear down the session. + ExpectAndFailAcquireNotify(connection_ptr); + session_map.at(kClienT0Pid).handle->AcquireRequest(); + + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.empty(); + })) << "Timed out waiting for session teardown after non-ENOBUFS failure"; + } + + ExpectServerDestruction(); + UninstantiateServer(); +} + +TEST_F(MessagePassingServerFixture, DisconnectDuringNotifyShouldNotCrash) +{ + ExpectOurPidIsQueried(); + + InstantiateServer(GetCountingSessionFactory()); + + auto* connection_ptr = ConnectClientAndSendConnectMessage(kClienT0Pid); + + // Simulate a disconnect arriving right after the Notify() kernel call. + // In production the disconnect callback fires on the dispatch thread and can + // race with NotifyAcquireRequest. NotifyAcquireRequest holds the server mutex + // for the duration of Notify(), so we spawn the disconnect thread inside the + // mock (it blocks on the mutex) and join it after AcquireRequest returns. + std::thread disconnect_thread; + EXPECT_CALL(*connection_ptr, Notify(Matcher>(_))) + .WillOnce([this, connection_ptr, &disconnect_thread](const auto /*m*/) { + disconnect_thread = std::thread([this, connection_ptr]() { + this->disconnect_callback(*connection_ptr); + }); + return score::cpp::expected_blank{}; + }); + + session_map.at(kClienT0Pid).handle->AcquireRequest(); + disconnect_thread.join(); + + // Wait for teardown to complete. + { + std::unique_lock lock(map_mutex); + ASSERT_TRUE(map_cond.wait_for(lock, std::chrono::seconds{1}, [this]() { + return session_map.empty(); + })) << "Timed out waiting for session teardown after disconnect-during-notify"; + } + + ExpectServerDestruction(); + UninstantiateServer(); +} TEST(MessagePassingServerTests, sessionWrapperCreateTest) { InSequence s; @@ -680,25 +919,10 @@ TEST(MessagePassingServerTests, sessionWrapperCreateTest) TEST(MessagePassingServerTests, sessionHandleCreateTest) { const pid_t pid = 0; - - auto client = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); - - auto* client_raw_ptr = client.get(); MessagePassingServer* msg_server = nullptr; - EXPECT_CALL(*client_raw_ptr, - Start(Matcher(_), - Matcher(_))); - - EXPECT_CALL(*client_raw_ptr, GetState()) - .WillRepeatedly(Return(score::message_passing::IClientConnection::State::kReady)); - - EXPECT_CALL(*client_raw_ptr, Send(An>())).Times(1); - - MessagePassingServer::SessionHandle session_handle(pid, msg_server, std::move(client)); - - EXPECT_NO_FATAL_FAILURE(session_handle.AcquireRequest()); - EXPECT_CALL(*client_raw_ptr, Destruct()).Times(AnyNumber()); + MessagePassingServer::SessionHandle session_handle(pid, msg_server); + EXPECT_FALSE(session_handle.AcquireRequest()); } struct TestParams @@ -865,25 +1089,12 @@ TEST_F(MessagePassingServerFixture, MessageCallbackUnknownMessageType) TEST(MessagePassingServerTests, SessionHandleAcquireRequestNotReady) { const pid_t pid = 0; - - auto client = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); - auto* client_raw_ptr = client.get(); MessagePassingServer* msg_server = nullptr; - EXPECT_CALL(*client_raw_ptr, - Start(Matcher(_), - Matcher(_))); - - // Return a non-Ready state to trigger "return false" - EXPECT_CALL(*client_raw_ptr, GetState()) - .WillRepeatedly(Return(score::message_passing::IClientConnection::State::kStarting)); - - MessagePassingServer::SessionHandle session_handle(pid, msg_server, std::move(client)); + MessagePassingServer::SessionHandle session_handle(pid, msg_server); const bool result = session_handle.AcquireRequest(); EXPECT_FALSE(result); - - EXPECT_CALL(*client_raw_ptr, Destruct()).Times(AnyNumber()); } // Covers OnConnectRequest "ConnectMessageFromClient too small" branch: @@ -978,17 +1189,6 @@ TEST_F(MessagePassingServerFixture, OnConnectRequestStopRequestedCoversEarlyExit auto* server_for_test = &(*server); server_for_test->stop_source_.request_stop(); - // client_factory_->Create() is called before the stop check — set up the mock - auto client = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - auto* client_mock = client.get(); - EXPECT_CALL(*client_factory_mock, - Create(Matcher(_), - Matcher(_))) - .WillOnce(Return(ByMove(std::move(client)))); - // sender is destroyed at the early return inside OnConnectRequest - EXPECT_CALL(*client_mock, Destruct()).Times(1); - StrictMock<::score::message_passing::ServerConnectionMock> connection; score::message_passing::ClientIdentity client_identity{kClienT0Pid, 0, 0}; EXPECT_CALL(connection, GetClientIdentity()).Times(AnyNumber()).WillRepeatedly(ReturnRef(client_identity)); diff --git a/score/datarouter/test/utils/data_router_test_utils.h b/score/datarouter/test/utils/data_router_test_utils.h index 02a5ac1e..faabd541 100644 --- a/score/datarouter/test/utils/data_router_test_utils.h +++ b/score/datarouter/test/utils/data_router_test_utils.h @@ -16,7 +16,7 @@ #include "score/datarouter/datarouter/data_router.h" -#include "score/static_reflection_with_serialization/serialization/include/serialization/for_logging.h" +#include "common/serialization/include/serialization/for_logging.h" namespace test::utils { diff --git a/score/mw/log/detail/data_router/data_router_message_client_factory_test.cpp b/score/mw/log/detail/data_router/data_router_message_client_factory_test.cpp index fc7564e5..816c2798 100644 --- a/score/mw/log/detail/data_router/data_router_message_client_factory_test.cpp +++ b/score/mw/log/detail/data_router/data_router_message_client_factory_test.cpp @@ -41,7 +41,6 @@ using ::testing::StrEq; const auto kMwsrFileName = ""; constexpr pid_t kThisProcessPid = 1234; constexpr uid_t kUid = 1234; -const std::string kClientReceiverIdentifier = "/" + std::string(""); class DatarouterMessageClientFactoryFixture : public ::testing::Test { @@ -111,8 +110,6 @@ TEST_F(DatarouterMessageClientFactoryFixture, CreateOnceShouldReturnClientWithEx auto* client_impl = dynamic_cast(client.get()); // Using the getters check that the factory provided the expected values to the constructor. - - EXPECT_EQ(client_impl->GetReceiverIdentifier(), kClientReceiverIdentifier); EXPECT_EQ(client_impl->GetAppid(), LoggingIdentifier{config_.GetAppId()}); EXPECT_EQ(client_impl->GetThisProcessPid(), kThisProcessPid); EXPECT_EQ(client_impl->GetWriterFileName(), kMwsrFileName); diff --git a/score/mw/log/detail/data_router/data_router_message_client_impl.cpp b/score/mw/log/detail/data_router/data_router_message_client_impl.cpp index 26fa28f9..b16454c6 100644 --- a/score/mw/log/detail/data_router/data_router_message_client_impl.cpp +++ b/score/mw/log/detail/data_router/data_router_message_client_impl.cpp @@ -19,10 +19,9 @@ #include "score/mw/log/detail/error.h" #include "score/mw/log/detail/initialization_reporter.h" -#include "score/os/utils/signal_impl.h" -#include "score/mw/log/detail/utils/signal_handling/signal_handling.h" #include #include +#include namespace score { @@ -52,7 +51,6 @@ DatarouterMessageClientImpl::DatarouterMessageClientImpl(const MsgClientIdentifi state_condition_{}, sender_state_{}, sender_{nullptr}, - receiver_{nullptr}, connect_thread_{} { } @@ -68,7 +66,6 @@ void DatarouterMessageClientImpl::Run() // coverity[autosar_cpp14_a15_4_2_violation] SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(run_started_ == false, "Run() must be called only once"); run_started_ = true; - SetupReceiver(); RunConnectTask(); } @@ -94,7 +91,7 @@ void DatarouterMessageClientImpl::ConnectToDatarouter() noexcept return; } - // Wait for the sender to be in Ready state before starting receiver + // Wait for the sender to be in Ready state before sending connect message { std::unique_lock lock(sender_state_change_mutex_); state_condition_.wait(lock, [&stop_source = stop_source_, &sender_state = sender_state_]() { @@ -123,12 +120,6 @@ void DatarouterMessageClientImpl::ConnectToDatarouter() noexcept } // LCOV_EXCL_STOP - if (StartReceiver() == false) - { - RequestInternalShutdown(); - return; - } - CheckExitRequestAndSendConnectMessage(); } @@ -181,73 +172,6 @@ void DatarouterMessageClientImpl::SetThreadName() noexcept } } -void DatarouterMessageClientImpl::SetupReceiver() noexcept -{ - const score::message_passing::ServiceProtocolConfig service_protocol_config{ - msg_client_ids_.GetReceiverID(), MessagePassingConfig::kMaxMessageSize, 0U, 0U}; - - constexpr score::message_passing::IServerFactory::ServerConfig kServerConfig{ - MessagePassingConfig::kMaxReceiverQueueSize, 0U, 0U}; - receiver_ = message_passing_factory_->CreateServer(service_protocol_config, kServerConfig); -} - -bool DatarouterMessageClientImpl::StartReceiver() -{ - // When the receiver starts listening, receive callbacks may be called that use the sender to reply. - // Thus we must create the sender before starting to listen to messages. - // Note that the receiver callback may only be called after the connect task finished. - SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(sender_ != nullptr, "The sender must be created before the receiver."); - - auto* this_ptr = this; - auto connect_callback = [this_ptr](score::message_passing::IServerConnection& connection) noexcept -> std::uintptr_t { - const auto result = SignalHandling::PThreadBlockSigTerm(this_ptr->utils_.GetSignal()); - static_cast(result); - const pid_t client_pid = connection.GetClientIdentity().pid; - return static_cast(client_pid); - }; - auto disconnect_callback = [this_ptr](score::message_passing::IServerConnection& /*connection*/) noexcept { - this_ptr->RequestInternalShutdown(); - }; - auto received_send_message_callback = [this_ptr]( - score::message_passing::IServerConnection& /*connection*/, - const score::cpp::span /*message*/) noexcept -> score::cpp::blank { - this_ptr->OnAcquireRequest(); - return {}; - }; - auto received_send_message_with_reply_callback = - [](score::message_passing::IServerConnection& /*connection*/, - score::cpp::span /*message*/) noexcept -> score::cpp::blank { - return {}; - }; - - const auto result = receiver_->StartListening(connect_callback, - disconnect_callback, - received_send_message_callback, - received_send_message_with_reply_callback); - - if (result.has_value() == false) - { - const std::string underlying_error = result.error().ToString(); - ReportInitializationError(mw::log::detail::Error::kReceiverInitializationError, - std::string_view{underlying_error}, - msg_client_ids_.GetAppID().GetStringView()); - - std::array app_zero_terminated{}; - std::ignore = std::copy_n(msg_client_ids_.GetAppID().GetStringView().begin(), - std::min(msg_client_ids_.GetAppID().GetStringView().size(), - app_zero_terminated.size() - static_cast(1)), - app_zero_terminated.begin()); - std::cerr - << "[[mw::log]] Application " << app_zero_terminated.data() << " (PID: " << msg_client_ids_.GetThisProcID() - << ") failed to start message passing receiver. Please add the 'PROCMGR_AID_PATHSPACE' ability to your" - "'app_config.json'." - << '\n'; - - return false; - } - return true; -} - void DatarouterMessageClientImpl::RequestInternalShutdown() noexcept { // Unlink the shared memory file as early as possible to prevent memory leaks. @@ -258,6 +182,11 @@ void DatarouterMessageClientImpl::RequestInternalShutdown() noexcept void DatarouterMessageClientImpl::CheckExitRequestAndSendConnectMessage() noexcept { + // LCOV_EXCL_START : Defensive check for the rare race between the stop_requested() guard in + // ConnectToDatarouter() and this call site. ConnectToDatarouter() already returns early when + // stop is requested, so reaching this function with stop_requested() == true requires stop to + // be requested in the narrow window between the two checks. That window is not deterministically + // coverable in a unit test. if (stop_source_.stop_requested()) { ReportInitializationError(score::mw::log::detail::Error::kShutdownDuringInitialization, @@ -265,6 +194,7 @@ void DatarouterMessageClientImpl::CheckExitRequestAndSendConnectMessage() noexce msg_client_ids_.GetAppID().GetStringView()); return; } + // LCOV_EXCL_STOP SendConnectMessage(); } @@ -323,7 +253,6 @@ void DatarouterMessageClientImpl::Shutdown() noexcept connect_thread_.join(); } - receiver_.reset(); { std::unique_lock lock(sender_mutex_); sender_.reset(); @@ -361,13 +290,36 @@ score::cpp::expected_blank DatarouterMessageClientImpl::Create sender_state_ = new_state; } state_condition_.notify_all(); + + if (new_state == score::message_passing::IClientConnection::State::kStopped) + { + RequestInternalShutdown(); + } }; + auto notify_callback = [this](score::cpp::span message) noexcept { + this->OnNotify(message); + }; // coverity[autosar_cpp14_a5_1_4_violation]: See justification above - sender_->Start(state_callback, score::message_passing::IClientConnection::NotifyCallback{}); + sender_->Start(state_callback, notify_callback); return {}; } +void DatarouterMessageClientImpl::OnNotify(const score::cpp::span message) noexcept +{ + if (message.size() != 1U) + { + return; + } + + if (message.front() != score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireRequest)) + { + return; + } + + OnAcquireRequest(); +} + void DatarouterMessageClientImpl::OnAcquireRequest() noexcept { // The acquire request shall be the first message Datarouter sends to the client. @@ -433,11 +385,6 @@ void DatarouterMessageClientImpl::UnlinkSharedMemoryFile() noexcept } } -const std::string& DatarouterMessageClientImpl::GetReceiverIdentifier() const noexcept -{ - return msg_client_ids_.GetReceiverID(); -} - const pid_t& DatarouterMessageClientImpl::GetThisProcessPid() const noexcept { return msg_client_ids_.GetThisProcID(); diff --git a/score/mw/log/detail/data_router/data_router_message_client_impl.h b/score/mw/log/detail/data_router/data_router_message_client_impl.h index 5109ed2b..daf21a32 100644 --- a/score/mw/log/detail/data_router/data_router_message_client_impl.h +++ b/score/mw/log/detail/data_router/data_router_message_client_impl.h @@ -18,7 +18,6 @@ #include "score/jthread.hpp" #include "score/optional.hpp" #include "score/stop_token.hpp" -#include "score/message_passing/i_server_connection.h" #include "score/os/errno.h" #include "score/mw/log/detail/data_router/data_router_message_client.h" #include "score/mw/log/detail/data_router/data_router_message_client_backend.h" @@ -70,28 +69,21 @@ class DatarouterMessageClientImpl : public DatarouterMessageClient /// \pre Shall not be called concurrently to Run(). void Shutdown() noexcept override; - void SetupReceiver() noexcept; - /// \brief Creates the message passing client (sender) for communication with Datarouter. /// \returns An empty expected on success, or an error if the sender could not be created. score::cpp::expected_blank CreateSender() noexcept; - /// \pre SetupReceiver and CreateSender() called before. - /// \returns true if the receiver was started successfully. - bool StartReceiver(); - /// \pre CreateSender() called before. void SendConnectMessage() noexcept; /// \brief Sets the thread name of the logger thread. void SetThreadName() noexcept; - /// \pre SetupReceiver() called before. + /// \brief Connects to the Datarouter by creating the sender and sending the connect message. void ConnectToDatarouter() noexcept; void BlockTermSignal() const noexcept; - const std::string& GetReceiverIdentifier() const noexcept; const pid_t& GetThisProcessPid() const noexcept; const std::string& GetWriterFileName() const noexcept; const LoggingIdentifier& GetAppid() const noexcept; @@ -99,6 +91,7 @@ class DatarouterMessageClientImpl : public DatarouterMessageClient private: void RunConnectTask(); void OnAcquireRequest() noexcept; + void OnNotify(score::cpp::span message) noexcept; void UnlinkSharedMemoryFile() noexcept; void HandleFirstMessageReceived() noexcept; void RequestInternalShutdown() noexcept; @@ -129,14 +122,12 @@ class DatarouterMessageClientImpl : public DatarouterMessageClient score::cpp::optional sender_state_; // The construction/destruction order is critical here! - // Sender and receiver both may contain running tasks. - // Receiver tasks (callbacks) may use the sender. - // Thus the receiver needs to destruct first, and then the sender. - // Finally it is safe to join the connect thread. + // The sender may contain running tasks. + // The connect thread may use the sender. + // Thus the sender needs to destruct before joining the connect thread. // Only then we can ensure that there are no concurrent tasks // accessing private data from another thread. score::cpp::pmr::unique_ptr sender_; - score::cpp::pmr::unique_ptr receiver_; score::cpp::jthread connect_thread_; }; diff --git a/score/mw/log/detail/data_router/data_router_message_client_test.cpp b/score/mw/log/detail/data_router/data_router_message_client_test.cpp index 38704437..4bee1149 100644 --- a/score/mw/log/detail/data_router/data_router_message_client_test.cpp +++ b/score/mw/log/detail/data_router/data_router_message_client_test.cpp @@ -13,9 +13,6 @@ #include "score/assert_support.hpp" #include "score/message_passing/mock/client_connection_mock.h" -#include "score/message_passing/mock/server_connection_mock.h" -#include "score/message_passing/mock/server_mock.h" -#include "score/message_passing/server_types.h" #include "score/os/mocklib/mock_pthread.h" #include "score/os/mocklib/unistdmock.h" #include "score/os/utils/mocklib/signalmock.h" @@ -43,7 +40,6 @@ using ::testing::_; using ::testing::ByMove; using ::testing::Matcher; using ::testing::Return; -using ::testing::ReturnRef; using ::testing::StrEq; class DatarouterMessageClientMockTest : public ::testing::Test @@ -73,17 +69,6 @@ MATCHER_P(CompareServiceProtocol, expected, "") return true; } -MATCHER_P(CompareServerConfig, expected, "") -{ - if (arg.max_queued_sends != expected.max_queued_sends || - arg.pre_alloc_connections != expected.pre_alloc_connections || - arg.max_queued_notifies != expected.max_queued_notifies) - { - return false; - } - return true; -} - MATCHER_P(CompareClientConfig, expected, "") { if (arg.max_async_replies != expected.max_async_replies || arg.max_queued_sends != expected.max_queued_sends || @@ -105,7 +90,7 @@ constexpr pthread_t kThreadId = 42; constexpr auto kLoggerThreadName = "logger"; constexpr uid_t kDatarouterDummyUid = 111; constexpr std::uint32_t kMaxSendBytes{17U}; -constexpr std::uint32_t kMaxNumberMessagesInReceiverQueue{0UL}; +constexpr std::uint32_t kMaxNotifyBytes{1U}; class DatarouterMessageClientFixture : public ::testing::Test { @@ -169,70 +154,17 @@ class DatarouterMessageClientFixture : public ::testing::Test .WillOnce(Return(score::cpp::make_unexpected(score::os::Error::createUnspecifiedError()))); } - score::message_passing::ServerMock* ExpectReceiverCreated() - { - auto receiver = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - auto* receiver_ptr = receiver.get(); - const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kClientReceiverIdentifier, kMaxSendBytes, 0U, 0U}; - - const score::message_passing::IServerFactory::ServerConfig server_config{ - kMaxNumberMessagesInReceiverQueue, 0U, 0U}; - EXPECT_CALL(*message_passing_factory_, - CreateServer(CompareServiceProtocol(service_protocol_config), CompareServerConfig(server_config))) - .WillOnce(Return(ByMove(std::move(receiver)))); - return receiver_ptr; - } - - void ExpectReceiverStartListening( - score::message_passing::ServerMock* receiver_ptr, - score::message_passing::ConnectCallback* connect_callback = nullptr, - score::message_passing::DisconnectCallback* disconnect_callback = nullptr, - score::message_passing::MessageCallback* sent_callback = nullptr, - score::message_passing::MessageCallback* sent_with_reply_callback = nullptr, - score::cpp::expected_blank result = score::cpp::expected_blank{}) - { - EXPECT_CALL(*receiver_ptr, - StartListening(Matcher(_), - Matcher(_), - Matcher(_), - Matcher(_))) - .WillOnce([connect_callback, disconnect_callback, sent_callback, sent_with_reply_callback, result]( - score::message_passing::ConnectCallback con_callback, - score::message_passing::DisconnectCallback discon_callback, - score::message_passing::MessageCallback sn_callback, - score::message_passing::MessageCallback sn_rep_callback) { - if (connect_callback != nullptr) - { - *connect_callback = std::move(con_callback); - } - if (disconnect_callback != nullptr) - { - *disconnect_callback = std::move(discon_callback); - } - if (sent_callback != nullptr) - { - *sent_callback = std::move(sn_callback); - } - if (sent_with_reply_callback != nullptr) - { - *sent_with_reply_callback = std::move(sn_rep_callback); - } - return result; - }); - } - score::message_passing::ClientConnectionMock* ExpectSenderCreation( score::message_passing::IClientConnection::StateCallback* state_callback = nullptr, - std::promise* callback_registered = nullptr) + std::promise* callback_registered = nullptr, + score::message_passing::IClientConnection::NotifyCallback* notify_callback = nullptr) { sender_mock_in_transit_ = score::cpp::pmr::make_unique>( score::cpp::pmr::get_default_resource()); auto* sender_mock = sender_mock_in_transit_.get(); const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; + kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, kMaxNotifyBytes}; const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; @@ -243,13 +175,17 @@ class DatarouterMessageClientFixture : public ::testing::Test EXPECT_CALL(*sender_mock, Start(Matcher(_), Matcher(_))) - .WillOnce([state_callback, callback_registered]( + .WillOnce([state_callback, callback_registered, notify_callback]( score::message_passing::IClientConnection::StateCallback st_callback, - score::message_passing::IClientConnection::NotifyCallback) { + score::message_passing::IClientConnection::NotifyCallback nt_callback) { if (state_callback != nullptr) { *state_callback = std::move(st_callback); } + if (notify_callback != nullptr) + { + *notify_callback = std::move(nt_callback); + } if (callback_registered != nullptr) { callback_registered->set_value(); @@ -264,10 +200,6 @@ class DatarouterMessageClientFixture : public ::testing::Test EXPECT_CALL(*sender_mock, Destruct()); } - void ExpectServerDestruction(score::message_passing::ServerMock* receiver_mock) - { - EXPECT_CALL(*receiver_mock, Destruct()); - } void ExpectSendAcquireResponse( score::message_passing::ClientConnectionMock* sender_ptr, ReadAcquireResult expected_content, @@ -319,49 +251,32 @@ class DatarouterMessageClientFixture : public ::testing::Test }); } - void SendAcquireRequestAndExpectResponse(score::message_passing::MessageCallback& acquire_callback, - score::message_passing::ClientConnectionMock** sender_ptr, - bool first_message, - bool unlink_successful = true) + void SendAcquireNotifyAndExpectResponse(score::message_passing::IClientConnection::NotifyCallback& notify_callback, + score::message_passing::ClientConnectionMock** sender_ptr, + bool first_message, + bool unlink_successful = true) { ReadAcquireResult acquired_data{}; acquired_data.acquired_buffer = shared_data_.control_block.switch_count_points_active_for_writing.load(); if (first_message) { - // MwsrWriter file shall be unlinked on first acquire request. ExpectUnlinkMwsrWriterFile(unlink_successful); } ExpectSendAcquireResponse(*sender_ptr, acquired_data); - score::message_passing::ServerConnectionMock connection; - const score::cpp::span message{}; - acquire_callback(connection, message); + const std::array message{score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireRequest)}; + notify_callback(message); } - void ExpectSenderAndReceiverCreation( - score::message_passing::ServerMock** receiver_ptr, + void ExpectSenderCreationSequence( score::message_passing::ClientConnectionMock** sender_ptr, - score::message_passing::IClientConnection::StateCallback* state_callback = nullptr, std::promise* callback_registered = nullptr, - score::cpp::expected_blank listen_result = score::cpp::expected_blank{}, - score::message_passing::ConnectCallback* connect_callback = nullptr, - score::message_passing::DisconnectCallback* disconnect_callback = nullptr, - score::message_passing::MessageCallback* sent_callback = nullptr, - score::message_passing::MessageCallback* sent_with_reply_callback = nullptr, bool block_termination_signal_pass = true, - bool receiver_start_listening = true) - + score::message_passing::IClientConnection::NotifyCallback* notify_callback = nullptr) { - std::ignore = block_termination_signal_pass; // TODO: remove this param - auto* receiver = ExpectReceiverCreated(); - if (receiver_ptr != nullptr) - { - *receiver_ptr = receiver; - } - if (block_termination_signal_pass) { ExpectBlockTerminationSignalPass(); @@ -373,20 +288,11 @@ class DatarouterMessageClientFixture : public ::testing::Test ExpectSetLoggerThreadName(); - auto* sender = ExpectSenderCreation(state_callback, callback_registered); + auto* sender = ExpectSenderCreation(state_callback, callback_registered, notify_callback); if (sender_ptr != nullptr) { *sender_ptr = sender; } - if (receiver_start_listening) - { - ExpectReceiverStartListening(receiver, - connect_callback, - disconnect_callback, - sent_callback, - sent_with_reply_callback, - listen_result); - } } void ExpectSendConnectMessage(score::message_passing::ClientConnectionMock* sender_ptr) @@ -424,16 +330,12 @@ class DatarouterMessageClientFixture : public ::testing::Test EXPECT_CALL(*pthread_mock_, setname_np(kThreadId, StrEq(kLoggerThreadName))).WillOnce(Return(setname_result)); } - void ExecuteCreateSenderAndReceiverSequence( - bool expect_receiver_success = true, - score::message_passing::IClientConnection::StateCallback* state_callback = nullptr) + void ExecuteCreateSenderSequence(score::message_passing::IClientConnection::StateCallback* state_callback = nullptr) { - client_->SetupReceiver(); client_->BlockTermSignal(); client_->SetThreadName(); client_->CreateSender(); (*state_callback)(score::message_passing::IClientConnection::State::kReady); - EXPECT_EQ(client_->StartReceiver(), expect_receiver_success); } bool unlink_done_{false}; @@ -484,58 +386,6 @@ TEST_F(DatarouterMessageClientFixture, CreateSenderShouldCreateSenderWithExpecte client_->CreateSender(); } -TEST_F(DatarouterMessageClientFixture, StartReceiverShouldStartListenSuccessfully) -{ - RecordProperty("ASIL", "B"); - RecordProperty("Description", "Verifies creating the receiver works properly."); - RecordProperty("TestType", "Interface test"); - RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); - - testing::InSequence order_matters; - - score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - score::message_passing::IClientConnection::StateCallback state_callback; - - ExpectSenderAndReceiverCreation(&receiver_ptr, &sender_ptr, &state_callback); - - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); - ExecuteCreateSenderAndReceiverSequence(true, &state_callback); -} - -TEST_F(DatarouterMessageClientFixture, StartReceiverWithoutSenderAndReceiverShouldFail) -{ - RecordProperty("ASIL", "B"); - RecordProperty("Description", "Verifies creating the receiver works properly."); - RecordProperty("TestType", "Interface test"); - RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); - - testing::InSequence order_matters; - - EXPECT_DEATH(client_->StartReceiver(), ""); -} -TEST_F(DatarouterMessageClientFixture, ReceiverStartListeningFailsShouldBeHandledGracefully) -{ - RecordProperty("ASIL", "B"); - RecordProperty("Description", "Verifies the ability of handling the receiver listen failure properly."); - RecordProperty("TestType", "Interface test"); - RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); - - testing::InSequence order_matters; - - score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - score::message_passing::IClientConnection::StateCallback state_callback; - - auto start_listening_error = score::cpp::make_unexpected(score::os::Error::createFromErrno()); - ExpectSenderAndReceiverCreation(&receiver_ptr, &sender_ptr, &state_callback, nullptr, start_listening_error); - - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); - ExecuteCreateSenderAndReceiverSequence(false, &state_callback); -} - TEST_F(DatarouterMessageClientFixture, SendConnectMessageShouldSendExpectedPayload) { RecordProperty("ASIL", "B"); @@ -606,14 +456,11 @@ TEST_F(DatarouterMessageClientFixture, ConnectToDatarouterShouldSendConnectMessa testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - ExpectSenderAndReceiverCreation(&receiver_ptr, &sender_ptr, &state_callback, &callback_registered); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, &callback_registered); ExpectSendConnectMessage(sender_ptr); - ExpectServerDestruction(receiver_ptr); ExpectClientDestruction(sender_ptr); - client_->SetupReceiver(); // We need to unblock waiting for the connection so we change the state in a separate thread std::thread connect_thread([this]() noexcept { client_->ConnectToDatarouter(); @@ -625,116 +472,88 @@ TEST_F(DatarouterMessageClientFixture, ConnectToDatarouterShouldSendConnectMessa connect_thread.join(); } -TEST_F(DatarouterMessageClientFixture, ConnectToDatarouterGivenThatReceiverFailedShouldNotSendConnectMessage) +TEST_F(DatarouterMessageClientFixture, AcquireNotifyShouldSendExpectedAcquireResponse) { RecordProperty("ASIL", "B"); - RecordProperty("Description", "Verifies the in-ability of sending connect message when receiver fails."); + RecordProperty( + "Description", + "Verifies that an acquire request delivered via NotifyCallback sends the expected acquire response."); RecordProperty("TestType", "Interface test"); RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; score::message_passing::IClientConnection::StateCallback state_callback; - std::promise callback_registered; - - ExpectSenderAndReceiverCreation(&receiver_ptr, - &sender_ptr, - &state_callback, - &callback_registered, - score::cpp::make_unexpected(score::os::Error::createFromErrno())); + score::message_passing::IClientConnection::NotifyCallback notify_callback; - ExpectUnlinkMwsrWriterFile(); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, nullptr, true, ¬ify_callback); - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); - client_->SetupReceiver(); - // We need to unblock waiting for the connection so we change the state in a separate thread - std::thread connect_thread([this]() noexcept { - client_->ConnectToDatarouter(); - }); + ExecuteCreateSenderSequence(&state_callback); - callback_registered.get_future().wait(); - state_callback(score::message_passing::IClientConnection::State::kReady); + bool first_message = true; + SendAcquireNotifyAndExpectResponse(notify_callback, &sender_ptr, first_message, false); - connect_thread.join(); + ExpectClientDestruction(sender_ptr); } -TEST_F(DatarouterMessageClientFixture, AcquireRequestShouldSendExpectedAcquireResponse) +TEST_F(DatarouterMessageClientFixture, InvalidNotifyShouldNotSendAcquireResponse) { RecordProperty("ASIL", "B"); - RecordProperty("Description", "Verifies that acquired request shall send the expected acquired response."); + RecordProperty("Description", "Verifies that invalid NotifyCallback payloads are ignored."); RecordProperty("TestType", "Interface test"); RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - score::message_passing::ConnectCallback connect_callback; - score::message_passing::DisconnectCallback disconnect_callback; - score::message_passing::MessageCallback sent_callback; - score::message_passing::MessageCallback sent_with_reply_callback; score::message_passing::IClientConnection::StateCallback state_callback; + score::message_passing::IClientConnection::NotifyCallback notify_callback; - ExpectSenderAndReceiverCreation(&receiver_ptr, - &sender_ptr, - &state_callback, - nullptr, - {}, - &connect_callback, - &disconnect_callback, - &sent_callback, - &sent_with_reply_callback); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, nullptr, true, ¬ify_callback); - ExecuteCreateSenderAndReceiverSequence(true, &state_callback); + ExecuteCreateSenderSequence(&state_callback); + + EXPECT_CALL(*sender_ptr, Send(Matcher>(_))).Times(0); + + const score::cpp::span empty_message{}; + notify_callback(empty_message); + + const std::array oversized_message{ + score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireRequest), std::uint8_t{0U}}; + notify_callback(oversized_message); + + const std::array wrong_id_message{score::cpp::to_underlying(DatarouterMessageIdentifier::kConnect)}; + notify_callback(wrong_id_message); - bool first_message = true; - SendAcquireRequestAndExpectResponse(sent_callback, &sender_ptr, first_message, false); - ExpectServerDestruction(receiver_ptr); ExpectClientDestruction(sender_ptr); } -TEST_F(DatarouterMessageClientFixture, SecondAcquireRequestShouldNotSetMwsrReader) +TEST_F(DatarouterMessageClientFixture, SecondAcquireNotifyShouldNotUnlinkMwsrWriter) { RecordProperty("ASIL", "B"); - RecordProperty("Description", "Verifies that the second acquire request should not set mwsr reader."); + RecordProperty("Description", "Verifies that the second acquire notify should not unlink the mwsr writer file."); RecordProperty("TestType", "Interface test"); RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - score::message_passing::ConnectCallback connect_callback; - score::message_passing::DisconnectCallback disconnect_callback; - score::message_passing::MessageCallback sent_callback; - score::message_passing::MessageCallback sent_with_reply_callback; score::message_passing::IClientConnection::StateCallback state_callback; + score::message_passing::IClientConnection::NotifyCallback notify_callback; - ExpectSenderAndReceiverCreation(&receiver_ptr, - &sender_ptr, - &state_callback, - nullptr, - {}, - &connect_callback, - &disconnect_callback, - &sent_callback, - &sent_with_reply_callback); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, nullptr, true, ¬ify_callback); - ExecuteCreateSenderAndReceiverSequence(true, &state_callback); + ExecuteCreateSenderSequence(&state_callback); bool first_message = true; - SendAcquireRequestAndExpectResponse(sent_callback, &sender_ptr, first_message, false); + SendAcquireNotifyAndExpectResponse(notify_callback, &sender_ptr, first_message, false); first_message = false; - SendAcquireRequestAndExpectResponse(sent_callback, &sender_ptr, first_message); - ExpectServerDestruction(receiver_ptr); + SendAcquireNotifyAndExpectResponse(notify_callback, &sender_ptr, first_message); ExpectClientDestruction(sender_ptr); } -// Refactor to acquire request TEST_F(DatarouterMessageClientFixture, ClientShouldShutdownAfterFailingToSendMessage) { RecordProperty("ASIL", "B"); @@ -745,23 +564,12 @@ TEST_F(DatarouterMessageClientFixture, ClientShouldShutdownAfterFailingToSendMes testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - score::message_passing::ConnectCallback connect_callback; - score::message_passing::DisconnectCallback disconnect_callback; - score::message_passing::MessageCallback sent_callback; - score::message_passing::MessageCallback sent_with_reply_callback; score::message_passing::IClientConnection::StateCallback state_callback; - ExpectSenderAndReceiverCreation(&receiver_ptr, - &sender_ptr, - &state_callback, - nullptr, - {}, - &connect_callback, - &disconnect_callback, - &sent_callback, - &sent_with_reply_callback); - - ExecuteCreateSenderAndReceiverSequence(true, &state_callback); + score::message_passing::IClientConnection::NotifyCallback notify_callback; + + ExpectSenderCreationSequence(&sender_ptr, &state_callback, nullptr, true, ¬ify_callback); + + ExecuteCreateSenderSequence(&state_callback); auto send_error = score::cpp::make_unexpected(score::os::Error::createFromErrno()); @@ -771,10 +579,8 @@ TEST_F(DatarouterMessageClientFixture, ClientShouldShutdownAfterFailingToSendMes ExpectUnlinkMwsrWriterFile(); ExpectSendAcquireResponse(sender_ptr, result, send_error); - score::message_passing::ServerConnectionMock connection; - const score::cpp::span message{}; - sent_callback(connection, message); - ExpectServerDestruction(receiver_ptr); + const std::array message{score::cpp::to_underlying(DatarouterMessageIdentifier::kAcquireRequest)}; + notify_callback(message); ExpectClientDestruction(sender_ptr); } @@ -788,14 +594,12 @@ TEST_F(DatarouterMessageClientFixture, RunShouldSetupAndConnect) testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; score::message_passing::IClientConnection::StateCallback state_callback; std::promise callback_registered; - ExpectSenderAndReceiverCreation(&receiver_ptr, &sender_ptr, &state_callback, &callback_registered); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, &callback_registered); ExpectSendConnectMessage(sender_ptr); - ExpectServerDestruction(receiver_ptr); ExpectClientDestruction(sender_ptr); ExpectUnlinkMwsrWriterFile(); client_->Run(); @@ -817,14 +621,12 @@ TEST_F(DatarouterMessageClientFixture, RunShallNotBeCalledMoreThanOnce) testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; score::message_passing::IClientConnection::StateCallback state_callback; std::promise callback_registered; - ExpectSenderAndReceiverCreation(&receiver_ptr, &sender_ptr, &state_callback, &callback_registered); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, &callback_registered); ExpectSendConnectMessage(sender_ptr); - ExpectServerDestruction(receiver_ptr); ExpectClientDestruction(sender_ptr); ExpectUnlinkMwsrWriterFile(); @@ -873,13 +675,11 @@ TEST_F(DatarouterMessageClientFixture, FailedToChownOwnMsrWriterFileForDataRoute testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; score::message_passing::IClientConnection::StateCallback state_callback; std::promise callback_registered; - ExpectSenderAndReceiverCreation(&receiver_ptr, &sender_ptr, &state_callback, &callback_registered); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, &callback_registered); ExpectSendConnectMessage(sender_ptr); - ExpectServerDestruction(receiver_ptr); ExpectClientDestruction(sender_ptr); ExpectUnlinkMwsrWriterFile(false); client_->Run(); @@ -900,24 +700,11 @@ TEST_F(DatarouterMessageClientFixture, GivenExitRequestDuringConnectionShouldNot testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; - - ExpectSenderAndReceiverCreation(&receiver_ptr, - &sender_ptr, - nullptr, - nullptr, - score::cpp::make_unexpected(score::os::Error::createFromErrno()), - nullptr, - nullptr, - nullptr, - nullptr, - true, - false); + + ExpectSenderCreationSequence(&sender_ptr); ExpectUnlinkMwsrWriterFile(); - ExpectServerDestruction(receiver_ptr); ExpectClientDestruction(sender_ptr); - client_->SetupReceiver(); stop_source_.request_stop(); client_->ConnectToDatarouter(); } @@ -932,26 +719,13 @@ TEST_F(DatarouterMessageClientFixture, FailedToEmptySignalSet) testing::InSequence order_matters; score::message_passing::ClientConnectionMock* sender_ptr{}; - score::message_passing::ServerMock* receiver_ptr{}; score::message_passing::IClientConnection::StateCallback state_callback; std::promise callback_registered; - ExpectSenderAndReceiverCreation(&receiver_ptr, - &sender_ptr, - &state_callback, - &callback_registered, - score::cpp::make_unexpected(score::os::Error::createFromErrno()), - nullptr, - nullptr, - nullptr, - nullptr, - false); - ExpectUnlinkMwsrWriterFile(); - - ExpectServerDestruction(receiver_ptr); + ExpectSenderCreationSequence(&sender_ptr, &state_callback, &callback_registered, false); + ExpectSendConnectMessage(sender_ptr); ExpectClientDestruction(sender_ptr); - client_->SetupReceiver(); // We need to unblock waiting for the connection so we change the state in a separate thread std::thread connect_thread([this]() noexcept { client_->ConnectToDatarouter(); @@ -964,17 +738,19 @@ TEST_F(DatarouterMessageClientFixture, FailedToEmptySignalSet) TEST_F(DatarouterMessageClientFixture, ConnectToDatarouterShouldReportErrorAndShutdownWhenCreateSenderFails) { + RecordProperty("ASIL", "B"); + RecordProperty("Description", + "Verifies ConnectToDatarouter reports an error and shuts down when CreateSender fails."); + RecordProperty("TestType", "Interface test"); + RecordProperty("DerivationTechnique", "Error guessing based on knowledge or experience"); testing::InSequence order_matters; - auto* receiver_ptr = ExpectReceiverCreated(); - ExpectBlockTerminationSignalPass(); ExpectSetLoggerThreadName(); - // CreateClient returns nullptr, causing CreateSender to return nullopt const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; + kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, kMaxNotifyBytes}; const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; EXPECT_CALL(*message_passing_factory_, @@ -982,275 +758,58 @@ TEST_F(DatarouterMessageClientFixture, ConnectToDatarouterShouldReportErrorAndSh .WillOnce(Return(ByMove(nullptr))); ExpectUnlinkMwsrWriterFile(); - ExpectServerDestruction(receiver_ptr); - client_->SetupReceiver(); client_->ConnectToDatarouter(); -} - -TEST_F(DatarouterMessageClientFixture, ConnectCallbackBlocksSignalAndReturnsClientPid) -{ - constexpr pid_t kExpectedPid = 42; - - auto* receiver_ptr = ExpectReceiverCreated(); - EXPECT_CALL(*signal_mock_, SigEmptySet(_)).Times(2).WillRepeatedly(Return(0)); - EXPECT_CALL(*signal_mock_, SigAddSet(_, SIGTERM)).Times(2).WillRepeatedly(Return(0)); - EXPECT_CALL(*signal_mock_, PthreadSigMask(_, _)).Times(2).WillRepeatedly(Return(0)); - ExpectSetLoggerThreadName(); - - sender_mock_in_transit_ = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - auto* sender_ptr = sender_mock_in_transit_.get(); - - const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; - const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; - - EXPECT_CALL(*message_passing_factory_, - CreateClient(CompareServiceProtocol(service_protocol_config), CompareClientConfig(client_config))) - .WillOnce(Return(ByMove(std::move(sender_mock_in_transit_)))); - - EXPECT_CALL(*sender_ptr, - Start(Matcher(_), - Matcher(_))) - .WillOnce([](score::message_passing::IClientConnection::StateCallback state_callback, - score::message_passing::IClientConnection::NotifyCallback) { - state_callback(score::message_passing::IClientConnection::State::kReady); - }); - - testing::StrictMock server_conn_mock; - - const score::message_passing::ClientIdentity expected_identity{kExpectedPid, 0, 0}; - EXPECT_CALL(server_conn_mock, GetClientIdentity()).WillOnce(ReturnRef(expected_identity)); - - // ExpectBlockTerminationSignalPass(); - - EXPECT_CALL(*receiver_ptr, - StartListening(Matcher(_), - Matcher(_), - Matcher(_), - Matcher(_))) - .WillOnce([&server_conn_mock](score::message_passing::ConnectCallback connect_cb, - score::message_passing::DisconnectCallback, - score::message_passing::MessageCallback, - score::message_passing::MessageCallback) { - auto result = connect_cb(server_conn_mock); - - EXPECT_TRUE(result.has_value()); - EXPECT_EQ(std::get(result.value()), static_cast(kExpectedPid)); - - return score::cpp::expected_blank{}; - }); - ExpectSendConnectMessage(sender_ptr); - - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); - - client_->SetupReceiver(); - client_->ConnectToDatarouter(); -} - -TEST_F(DatarouterMessageClientFixture, DisconnectCallbackRequestsInternalShutdown) -{ - auto* receiver_ptr = ExpectReceiverCreated(); - ExpectBlockTerminationSignalPass(); - ExpectSetLoggerThreadName(); - - sender_mock_in_transit_ = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - auto* sender_ptr = sender_mock_in_transit_.get(); - - const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; - const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; - - EXPECT_CALL(*message_passing_factory_, - CreateClient(CompareServiceProtocol(service_protocol_config), CompareClientConfig(client_config))) - .WillOnce(Return(ByMove(std::move(sender_mock_in_transit_)))); - - EXPECT_CALL(*sender_ptr, - Start(Matcher(_), - Matcher(_))) - .WillOnce([](score::message_passing::IClientConnection::StateCallback state_callback, - score::message_passing::IClientConnection::NotifyCallback) { - state_callback(score::message_passing::IClientConnection::State::kReady); - }); - - testing::StrictMock server_conn_mock; - - EXPECT_CALL(*receiver_ptr, - StartListening(Matcher(_), - Matcher(_), - Matcher(_), - Matcher(_))) - .WillOnce([&server_conn_mock](score::message_passing::ConnectCallback, - score::message_passing::DisconnectCallback disconnect_cb, - score::message_passing::MessageCallback, - score::message_passing::MessageCallback) { - disconnect_cb(server_conn_mock); - return score::cpp::expected_blank{}; - }); - - ExpectUnlinkMwsrWriterFile(); - - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); - - client_->SetupReceiver(); - client_->ConnectToDatarouter(); EXPECT_TRUE(stop_source_.stop_requested()); } -TEST_F(DatarouterMessageClientFixture, ReceivedSendMessageWithReplyCallbackDoesNothing) +TEST_F(DatarouterMessageClientFixture, CreateSenderFailsWhenFactoryReturnsNullptr) { - auto* receiver_ptr = ExpectReceiverCreated(); - - ExpectBlockTerminationSignalPass(); - ExpectSetLoggerThreadName(); + RecordProperty("ASIL", "B"); + RecordProperty("Description", + "Verifies CreateSender reports an error and returns unexpected when the factory returns nullptr."); + RecordProperty("TestType", "Interface test"); + RecordProperty("DerivationTechnique", "Error guessing based on knowledge or experience"); - sender_mock_in_transit_ = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - auto* sender_ptr = sender_mock_in_transit_.get(); + testing::InSequence order_matters; const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; + kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, kMaxNotifyBytes}; const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; + // Factory returns nullptr — this is the key to hitting the branch. EXPECT_CALL(*message_passing_factory_, CreateClient(CompareServiceProtocol(service_protocol_config), CompareClientConfig(client_config))) - .WillOnce(Return(ByMove(std::move(sender_mock_in_transit_)))); - - EXPECT_CALL(*sender_ptr, - Start(Matcher(_), - Matcher(_))) - .WillOnce([](score::message_passing::IClientConnection::StateCallback state_callback, - score::message_passing::IClientConnection::NotifyCallback) { - state_callback(score::message_passing::IClientConnection::State::kReady); - }); - - EXPECT_CALL(*sender_ptr, Send(Matcher>(_))) - .WillOnce(Return(score::cpp::expected_blank{})); - - testing::StrictMock server_conn_mock; - - EXPECT_CALL(*receiver_ptr, - StartListening(Matcher(_), - Matcher(_), - Matcher(_), - Matcher(_))) - .WillOnce([&server_conn_mock](score::message_passing::ConnectCallback, - score::message_passing::DisconnectCallback, - score::message_passing::MessageCallback, - score::message_passing::MessageCallback send_with_reply_cb) { - const std::array message_data{0x01, 0x02, 0x03, 0x04}; - const score::cpp::span message_span{message_data}; - - const auto result = send_with_reply_cb(server_conn_mock, message_span); - static_cast(result); - - return score::cpp::expected_blank{}; - }); + .WillOnce(Return(ByMove(nullptr))); - ExpectUnlinkMwsrWriterFile(); - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); + const auto result = client_->CreateSender(); - client_->SetupReceiver(); - client_->ConnectToDatarouter(); + ASSERT_FALSE(result.has_value()); } - -TEST_F(DatarouterMessageClientFixture, StopRequestedDuringReceiverStartReportsShutdownDuringInitialization) +TEST_F(DatarouterMessageClientFixture, StateCallbackKStoppedShouldTriggerInternalShutdown) { - auto* receiver_ptr = ExpectReceiverCreated(); + RecordProperty("ASIL", "QM"); + RecordProperty("Description", "Verifies dataRouter termination triggers internal shutdown on client side"); + RecordProperty("TestType", "Verification of the control flow and data flow"); + RecordProperty("DerivationTechnique", "Error guessing based on knowledge or experience"); - ExpectBlockTerminationSignalPass(); - ExpectSetLoggerThreadName(); - - sender_mock_in_transit_ = score::cpp::pmr::make_unique>( - score::cpp::pmr::get_default_resource()); - auto* sender_ptr = sender_mock_in_transit_.get(); - - const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; - const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; - - EXPECT_CALL(*message_passing_factory_, - CreateClient(CompareServiceProtocol(service_protocol_config), CompareClientConfig(client_config))) - .WillOnce(Return(ByMove(std::move(sender_mock_in_transit_)))); + testing::InSequence order_matters; - EXPECT_CALL(*sender_ptr, - Start(Matcher(_), - Matcher(_))) - .WillOnce([](score::message_passing::IClientConnection::StateCallback state_callback, - score::message_passing::IClientConnection::NotifyCallback) { - state_callback(score::message_passing::IClientConnection::State::kReady); - }); + score::message_passing::ClientConnectionMock* sender_ptr{}; + score::message_passing::IClientConnection::StateCallback state_callback; - // SendConnectMessage must never be reached - EXPECT_CALL(*sender_ptr, Send(Matcher>(_))).Times(0); + ExpectSenderCreationSequence(&sender_ptr, &state_callback); - testing::StrictMock server_conn_mock; - - EXPECT_CALL(*receiver_ptr, - StartListening(Matcher(_), - Matcher(_), - Matcher(_), - Matcher(_))) - .WillOnce([&server_conn_mock](score::message_passing::ConnectCallback, - score::message_passing::DisconnectCallback disconnect_cb, - score::message_passing::MessageCallback, - score::message_passing::MessageCallback) { - // Trigger disconnect during StartListening — this calls RequestInternalShutdown(), - // setting stop_source_ to stop_requested AFTER the earlier stop check in - // ConnectToDatarouter() has already passed. - disconnect_cb(server_conn_mock); - - return score::cpp::expected_blank{}; - }); + ExecuteCreateSenderSequence(&state_callback); ExpectUnlinkMwsrWriterFile(); - ExpectServerDestruction(receiver_ptr); - ExpectClientDestruction(sender_ptr); - client_->SetupReceiver(); - client_->ConnectToDatarouter(); + state_callback(score::message_passing::IClientConnection::State::kStopped); EXPECT_TRUE(stop_source_.stop_requested()); -} - -TEST_F(DatarouterMessageClientFixture, CreateSenderFailsWhenFactoryReturnsNullptr) -{ - auto* receiver_ptr = ExpectReceiverCreated(); - - ExpectBlockTerminationSignalPass(); - ExpectSetLoggerThreadName(); - - const score::message_passing::ServiceProtocolConfig service_protocol_config{ - kDatarouterReceiverIdentifier, kMaxSendBytes, 0U, 0U}; - const score::message_passing::IClientFactory::ClientConfig client_config{0, 10, false, true, false}; - - // Factory returns nullptr — this is the key to hitting the branch - EXPECT_CALL(*message_passing_factory_, - CreateClient(CompareServiceProtocol(service_protocol_config), CompareClientConfig(client_config))) - .WillOnce(Return(ByMove(nullptr))); - - // Since CreateSender fails, Start should never be called - // Since CreateSender fails, StartListening should never be called - EXPECT_CALL(*receiver_ptr, - StartListening(Matcher(_), - Matcher(_), - Matcher(_), - Matcher(_))) - .Times(0); - ExpectUnlinkMwsrWriterFile(); - ExpectServerDestruction(receiver_ptr); - - client_->SetupReceiver(); - client_->ConnectToDatarouter(); - - EXPECT_TRUE(stop_source_.stop_requested()); + ExpectClientDestruction(sender_ptr); } } // namespace diff --git a/score/mw/log/detail/data_router/message_passing_config.h b/score/mw/log/detail/data_router/message_passing_config.h index 7242bd99..17201377 100644 --- a/score/mw/log/detail/data_router/message_passing_config.h +++ b/score/mw/log/detail/data_router/message_passing_config.h @@ -32,20 +32,20 @@ struct MessagePassingConfig static constexpr std::uint32_t kMaxMessageSize{17U}; /// \brief Maximum number of messages in receiver queue. - /// \note Value not used at the moment of integration with message passing library. May change in the future. - static constexpr std::uint32_t kMaxReceiverQueueSize{0U}; + /// \note Value set based on the recommendation in message_passing/i_server_factory.h + static constexpr std::uint32_t kMaxReceiverQueueSize{1U}; /// \brief Number of pre-allocated connections. static constexpr std::uint32_t kPreAllocConnections{0U}; /// \brief Maximum number of queued notifications. - static constexpr std::uint32_t kMaxQueuedNotifies{0U}; + static constexpr std::uint32_t kMaxQueuedNotifies{1U}; /// \brief Maximum reply size in bytes. static constexpr std::uint32_t kMaxReplySize{0U}; /// \brief Maximum notification size in bytes. - static constexpr std::uint32_t kMaxNotifySize{0U}; + static constexpr std::uint32_t kMaxNotifySize{1U}; /// \brief The DataRouter receiver endpoint identifier. static constexpr const char* kDatarouterReceiverIdentifier{"/logging.datarouter_recv"};