feat(dispatcher): add event relay dispatcher and receiver modules - #256
Merged
Merged
Conversation
Reviewer's GuideIntroduces GIO/D-Bus-based relay receiver and UNIX socketpair-backed broadcast dispatcher modules, integrates their GLib/GIO build dependencies, and enables top-level CTest execution with broad dispatcher relay tests. Sequence diagram for D-Bus event relay channel setup and broadcastsequenceDiagram
participant Receiver
participant DBusService
participant Dispatcher
participant RelaySocket
Receiver->>DBusService: GetEventChannel()
DBusService->>Dispatcher: event_relay_dispatcher_get_event_channel()
Dispatcher->>RelaySocket: socketpair(AF_UNIX, SOCK_SEQPACKET)
Dispatcher-->>DBusService: client socket fd
DBusService-->>Receiver: D-Bus fd passing (fd, event_protocol_id)
Dispatcher->>RelaySocket: event_relay_dispatcher_send(buf, len)
RelaySocket-->>Receiver: event packet
Receiver->>RelaySocket: event_relay_receiver_receive(receiver, buf, len)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 6 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dispatcher/event_relay_dispatcher.c" line_range="109-113" />
<code_context>
+ if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ g_debug("relay: slow client fd %d, kicking", fd);
+ close(fd);
+ g_ptr_array_remove_index_fast(dispatcher->socket_list, i);
+ } else if (errno == EPIPE || errno == ECONNRESET) {
+ g_debug("relay: broken connection fd %d", fd);
</code_context>
<issue_to_address>
**issue (bug_risk):** The error paths close `fd` manually and then remove it from a `GPtrArray` whose destroy notify is `close_fd`, so the same descriptor is closed twice. This violates the ownership contract and can close an unrelated descriptor if the number is reused before the destroy notify runs.
**Triggers:** When sending to a slow or disconnected client.
**Suggested fix:** Remove the explicit `close(fd)` calls and let the array destroy notify close the descriptor, or remove the destroy notify ownership before closing manually.
</issue_to_address>
### Comment 2
<location path="CMakeLists.txt" line_range="16" />
<code_context>
+# dirs (each component registers its tests via its own ENABLE_TESTING).
+# Must precede add_subdirectory: testing support is only wired into
+# subdirectories added after this call.
+enable_testing()
add_subdirectory(${PROJECT_SOURCE_DIR}/src)
add_subdirectory(${PROJECT_SOURCE_DIR}/config)
</code_context>
<issue_to_address>
**issue (testing):** The top-level call enables CTest registration but does not set the component's `ENABLE_TESTING` option, which defaults to `OFF` in `src/dispatcher/CMakeLists.txt`; consequently the dispatcher test subdirectory is not added and a default top-level build produces no relay tests for `ctest` to run.
**Triggers:** With the normal default configuration that does not pass `-DENABLE_TESTING=ON`.
**Suggested fix:** Set the project testing option at the top level, for example with a top-level `option(ENABLE_TESTING ...)`, or conditionally add the component tests from that option.
```suggestion
option(ENABLE_TESTING "Enable testing" ON)
enable_testing()
```
</issue_to_address>
### Comment 3
<location path="src/dispatcher/event_relay_receiver.c" line_range="81-92" />
<code_context>
+
+ /* Extract the fd and event_protocol_id from the return value. */
+ guint32 protocol_id = 0;
+ g_variant_get(result, "(hu)", NULL, &protocol_id);
+ g_variant_unref(result);
+
+ if (fd_list == NULL || g_unix_fd_list_get_length(fd_list) < 1) {
+ g_warning("relay: GetEventChannel returned no file descriptor");
+ g_clear_object(&fd_list);
+ goto fail;
+ }
+
+ g_clear_error(&error);
+ gint fd = g_unix_fd_list_get(fd_list, 0, &error);
+ g_object_unref(fd_list);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The receiver discards the returned D-Bus handle from `(hu)` and always retrieves descriptor index 0 from `fd_list`; when the service returns a valid handle other than zero or includes multiple passed descriptors, the receiver reads the wrong descriptor.
**Triggers:** When `GetEventChannel` returns a handle that is not zero.
**Suggested fix:** Store the `h` value from `g_variant_get` and pass that handle to `g_unix_fd_list_get` instead of hard-coding 0.
```suggestion
guint32 protocol_id = 0;
gint32 fd_handle = -1;
g_variant_get(result, "(hu)", &fd_handle, &protocol_id);
g_variant_unref(result);
if (fd_list == NULL || g_unix_fd_list_get_length(fd_list) < 1) {
g_warning("relay: GetEventChannel returned no file descriptor");
g_clear_object(&fd_list);
goto fail;
}
g_clear_error(&error);
gint fd = g_unix_fd_list_get(fd_list, fd_handle, &error);
```
</issue_to_address>
### Comment 4
<location path="src/dispatcher/event_relay_receiver.c" line_range="151-164" />
<code_context>
+ g_return_val_if_fail(receiver != NULL, EVENT_RECEIVE_ERROR);
+ g_return_val_if_fail(buf != NULL, EVENT_RECEIVE_ERROR);
+
+ ssize_t ret = recv(receiver->sock_fd, buf, len, MSG_DONTWAIT);
+ if (ret < 0) {
+ if (errno == EINTR)
+ return EVENT_RECEIVE_INTERRUPTED;
+ if (errno == EAGAIN || errno == EWOULDBLOCK)
+ return EVENT_RECEIVE_WOULD_BLOCK;
+ g_debug("relay: recv() failed: %s", strerror(errno));
+ return EVENT_RECEIVE_ERROR;
+ }
+
+ if (ret == 0)
+ return EVENT_RECEIVE_DISCONNECTED;
+
+ return EVENT_RECEIVE_OK;
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** A `SOCK_SEQPACKET` message larger than `len` is truncated by `recv()` and the function still returns `EVENT_RECEIVE_OK`, so the caller cannot detect that the event payload was discarded or incomplete.
**Triggers:** When the sender transmits a packet larger than the caller's receive buffer.
**Suggested fix:** Use `MSG_TRUNC` or otherwise validate and report the received packet length before returning success.
</issue_to_address>
### Comment 5
<location path="src/dispatcher/event_relay_dispatcher.c" line_range="115-117" />
<code_context>
+ close(fd);
+ g_ptr_array_remove_index_fast(dispatcher->socket_list, i);
+ } else {
+ g_warning("relay: send() to fd %d failed: %s",
+ fd, strerror(errno));
+ }
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** For errors other than `EAGAIN`, `EWOULDBLOCK`, `EPIPE`, and `ECONNRESET`, `send()` failure is only logged while `event_relay_dispatcher_send()` returns `TRUE`; an `EINTR` or other critical failure therefore reports success even though that client did not receive the event.
**Triggers:** When `send()` is interrupted or fails with an unhandled errno.
**Suggested fix:** Retry `EINTR`, and return `FALSE` or remove/report the client for other failures according to the API contract.
</issue_to_address>
### Comment 6
<location path="src/dispatcher/tests/test_relay_dispatcher.c" line_range="269-271" />
<code_context>
+ /* The child may exit 0 (if g_warning wasn't fatal for some reason) or be
+ * killed by a signal from the fatal-warning handler (SIGABRT or SIGTRAP
+ * depending on the GLib backend). Accept either outcome. */
+ g_assert_true(WIFEXITED(status) || WIFSIGNALED(status));
+ if (WIFEXITED(status))
+ g_assert_cmpint(WEXITSTATUS(status), ==, 0);
+}
+
</code_context>
<issue_to_address>
**issue (testing):** The receiver error-path test accepts any child signal as success, so an assertion failure, crash, or unexpected abort in `event_relay_receiver_new()` makes the test pass instead of verifying that the constructor returns `NULL`.
**Triggers:** When the child terminates abnormally for a reason unrelated to the expected D-Bus failure.
**Suggested fix:** Require a normal exit with status 0, or explicitly validate only the expected fatal-warning signal and separately verify the constructor's return path without treating arbitrary crashes as success.
</issue_to_address>
Add event_relay_dispatcher and event_relay_receiver for GIO-based DBus event relay, and enable testing in top-level CMakeLists. 新增事件中继调度器与接收器模块，支持通过 DBus 进行事件转发。 Log: 新增事件中继调度器与接收器模块 PMS: BUG-370779 Influence: 新增 dispatcher 事件中继功能，支持 DBus 方式转发事件，并在顶层构建系统中启用 ctest。
wangrong1069
force-pushed
the
pr0911
branch
from
September 11, 2026 07:45
9b57767 to
872068d
Compare
max-lvs
approved these changes
Sep 11, 2026
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add event_relay_dispatcher and event_relay_receiver for GIO-based DBus event relay, and enable testing in top-level CMakeLists.
新增事件中继调度器与接收器模块，支持通过 DBus 进行事件转发。
Log: 新增事件中继调度器与接收器模块
PMS: BUG-370779
Influence: 新增 dispatcher 事件中继功能，支持 DBus 方式转发事件，并在顶层构建系统中启用 ctest。
Summary by Sourcery
Introduce DBus event relay components that broadcast dispatcher events to connected clients and enable their tests through the top-level build.
New Features:
Enhancements:
Build:
Tests: