Replace mmap-based IPC with score_baselibs SharedMemoryFactory in ts_client by gordon9901 · Pull Request #54 · eclipse-score/inc_time

gordon9901 · 2026-06-12T10:10:41Z

Summary

Replaces the hand-rolled POSIX shm_open/mmap implementation in ts_client
with score::memory::shared::SharedMemoryFactory from score_baselibs, and
wires up a new SvtHandlerShm timebase handler in TimeDaemon.

ts_client (`score/ts_client/`)

GptpIpcPublisher / GptpIpcReceiver: remove direct shm_open, mmap,
ftruncate, close calls; use SharedMemoryFactory::Create /
SharedMemoryFactory::Open instead. Memory region is now constructed via
ISharedMemoryResource::construct<GptpIpcRegion>().
SharedMemoryFactory::Remove + RemoveStaleArtefacts replace the raw
shm_unlink in publisher init.
Header guards updated to drop the redundant _SRC_ segment.

TimeDaemon (`score/time_daemon/src/application/svt/`)

New SvtHandlerShm (factory_shm.h/.cpp, svt_handler_shm.h/.cpp):
a TimebaseHandler implementation that reads live gPTP data from shared
memory via GPTPShmMachine and drives the same verification + IPC-publish
pipeline as the existing SvtHandler.

fixes (#41)

github-actions · 2026-06-12T10:11:33Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: ada07605-17ef-4e4b-8455-da9cc4cbfde0
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'aspect_rules_lint', the root module requires module version aspect_rules_lint@1.5.3, but got aspect_rules_lint@2.0.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Analyzing: target //:license-check (3 packages loaded)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (75 packages loaded, 9 targets configured)

Analyzing: target //:license-check (91 packages loaded, 9 targets configured)

Analyzing: target //:license-check (148 packages loaded, 2654 targets configured)

Analyzing: target //:license-check (157 packages loaded, 7827 targets configured)

Analyzing: target //:license-check (169 packages loaded, 8049 targets configured)

Analyzing: target //:license-check (169 packages loaded, 8049 targets configured)

Analyzing: target //:license-check (169 packages loaded, 8049 targets configured)

Analyzing: target //:license-check (169 packages loaded, 8049 targets configured)

Analyzing: target //:license-check (172 packages loaded, 9937 targets configured)

INFO: Analyzed target //:license-check (174 packages loaded, 10184 targets configured).
[9 / 16] [Prepa] Expanding template external/score_tooling+/dash/tool/formatters/_dash_format_converter.venv/lib/python3.12/site-packages/_bazel_site_init.py [for tool]
[13 / 16] Generating Dash formatted dependency file ...; 0s disk-cache, processwrapper-sandbox
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 2 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
[14 / 16] checking cached actions
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 32.782s, Critical Path: 2.25s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2026-06-12T10:15:24Z

The created documentation from the pull request is available at: docu-html

BjoernAtBosch · 2026-06-15T13:31:28Z

I think, we should not add these code duplications here. But instead replace the stub_ptp_machine by the shm_ptp_machine (see PR #25 how to achieve this)

The stub variant was kept to avoid breaking existing code during development. Now that the shm path is confirmed working, I've removed svt_handler_shm.cpp/.h and factory_shm.cpp/.h, and updated SvtHandler directly to use CreateGPTPShmMachine instead of CreateGPTPStubMachine.

BjoernAtBosch · 2026-06-15T13:32:40Z

 ********************************************************************************/
-#ifndef SCORE_TS_CLIENT_SRC_GPTP_IPC_H
-#define SCORE_TS_CLIENT_SRC_GPTP_IPC_H
+#ifndef SCORE_TS_CLIENT_GPTP_IPC_H


Here and everywhere: Why doing this include guard renaming?

The SRC segment encodes the physical directory path (ts_client/src/), which is an internal layout detail. Include guards should reflect the logical module name, not the filesystem structure — hence SCORE_TS_CLIENT_* rather than SCORE_TS_CLIENT_SRC_*. All 6 headers under ts_client/src/ were updated consistently.

Yes, I also see this. But I'm struggeling with the point, that the "physical" structure is also visible in the includes, e.g.

#include "score/ts_client/src/gptp_ipc_channel.h"

which then should also be

#include "score/ts_client/gptp_ipc_channel.h"

instead. But that's how it is in Bazel.

If we change it here, we would be inconsistent in the overall module and should change it everywhere. (Maybe not now)

Maybe there is some S-CORE convention. Lets check ...

BjoernAtBosch · 2026-06-15T13:34:17Z

 * @brief IPC data snapshot written by TimeSlave and read by TimeDaemon.
 *
- * This type is internal to ts_client/src and intentionally decoupled from
+ * This type is internal to libTSClient and intentionally decoupled from


Suggested change

* This type is internal to libTSClient and intentionally decoupled from

* This type is internal to ts_client and intentionally decoupled from

Good catch. This was an oversight — the comment should say ts_client, not libTSClient. Fixed.

BjoernAtBosch · 2026-06-15T13:36:18Z

 {

-/// Default POSIX shared memory name for the gPTP IPC channel.
 constexpr char kGptpIpcName[] = "/gptp_ptp_info";


Suggested change

constexpr char kGptpIpcName[] = "/gptp_ptp_info";

/// Default shared memory name for the gPTP IPC channel.

constexpr char kGptpIpcName[] = "/gptp_ptp_info";

Agreed. The comment was removed because "POSIX" is no longer accurate after switching to SharedMemoryFactory. Restored as /// Default shared memory name for the gPTP IPC channel.

BjoernAtBosch · 2026-06-15T13:38:42Z


-    /// Open and map the shared memory segment (read-only).
-    /// @return true on success.
    bool Init(const std::string& ipc_name = kGptpIpcName);


Suggested change

bool Init(const std::string& ipc_name = kGptpIpcName);

bool Open(const std::string& ipc_name = kGptpIpcName);

Agreed. Open/Close is more semantically precise and forms a natural pair. Renamed in both GptpIpcPublisher and GptpIpcReceiver, including all call sites and test cases.

BjoernAtBosch · 2026-06-15T13:39:42Z


-    /// Create and map the shared memory segment.
-    /// @return true on success.
    bool Init(const std::string& ipc_name = kGptpIpcName);


Suggested change

bool Init(const std::string& ipc_name = kGptpIpcName);

bool Open(const std::string& ipc_name = kGptpIpcName);

Agreed. Open/Close is more semantically precise and forms a natural pair. Renamed in both GptpIpcPublisher and GptpIpcReceiver, including all call sites and test cases.

BjoernAtBosch · 2026-06-15T13:40:01Z

    void Publish(const score::ts::GptpIpcData& data);
-
-    /// Unmap and unlink the shared memory segment.
    void Destroy();


Suggested change

void Destroy();

void Close();

Agreed. Open/Close is more semantically precise and forms a natural pair. Renamed in both GptpIpcPublisher and GptpIpcReceiver, including all call sites and test cases.

github-project-automation Bot added this to TIM - Time FT Jun 12, 2026

github-project-automation Bot moved this to In Progress in TIM - Time FT Jun 12, 2026

gordon9901 requested a deployment to workflow-approval June 12, 2026 10:10 — with GitHub Actions Waiting

Replace mmap-based IPC with SharedMemoryFactory in ts_client

7c692e8

gordon9901 force-pushed the ecarx_pr2_shm_impl branch from aad9e21 to 7c692e8 Compare June 12, 2026 10:17

gordon9901 requested a deployment to workflow-approval June 12, 2026 10:17 — with GitHub Actions Waiting

BjoernAtBosch suggested changes Jun 15, 2026

View reviewed changes

BlueXMarv3l requested a review from BjoernAtBosch June 17, 2026 08:49

rename Init/Destroy, drop svt_handler_shm

e14098e

gordon9901 requested a deployment to workflow-approval June 17, 2026 09:56 — with GitHub Actions Waiting

	* This type is internal to libTSClient and intentionally decoupled from
	* This type is internal to ts_client and intentionally decoupled from

	constexpr char kGptpIpcName[] = "/gptp_ptp_info";
	/// Default shared memory name for the gPTP IPC channel.
	constexpr char kGptpIpcName[] = "/gptp_ptp_info";

	bool Init(const std::string& ipc_name = kGptpIpcName);
	bool Open(const std::string& ipc_name = kGptpIpcName);

Conversation

gordon9901 commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

ts_client (score/ts_client/)

TimeDaemon (score/time_daemon/src/application/svt/)

Uh oh!

github-actions Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions Bot commented Jun 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BjoernAtBosch Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gordon9901 commented Jun 12, 2026 •

edited

Loading

ts_client (`score/ts_client/`)

TimeDaemon (`score/time_daemon/src/application/svt/`)

github-actions Bot commented Jun 12, 2026 •

edited

Loading

BjoernAtBosch Jun 17, 2026 •

edited

Loading