Added Os::CountingSemaphore osal module (#5173) by bevinduckett · Pull Request #5204 · nasa/fprime

bevinduckett · 2026-05-27T03:44:03Z


*Related Issue(s)*	#5173
*Has Unit Tests (y/n)*	Yes
*Documentation Included (y/n)*	N/A
*Generative AI was used in this contribution (y/n)*	yes

Change Description

Added Os::CountingSemaphore to the OS abstraction layer

Rationale

See rationale in #5173

Testing/Review Recommendations

I've verified all UTs pass & the reference deployment builds & runs.

Future Work

Once this is added to fprime, various OSs will need need to add CountingSemaphore s

AI Usage (see policy)

AI was used to assist with documentation, code review, and unit test implementation. The AI tool used was Claude Sonnet 4.5 - edits were reviewed by a human.

bevinduckett · 2026-05-27T17:54:56Z

@thomas-bc - could you kick off the other workflows?

    return status;
 }
+
+CountingSemaphore::Status posix_status_to_semaphore_status(int posix_status) {


+PosixCountingSemaphore::Status PosixCountingSemaphore::waitTimeout(U32 timeout_ms) {
+    FW_ASSERT(timeout_ms > 0);
+    struct timespec abstime;
+    int clock_status = clock_gettime(CLOCK_REALTIME, &abstime);


+}
+
+PosixCountingSemaphore::Status PosixCountingSemaphore::post() {
+    int status = sem_post(&this->m_handle.m_semaphore);


    return status;
 }
+
+CountingSemaphore::Status posix_status_to_semaphore_status(int posix_status) {


bevinduckett · 2026-05-27T22:46:07Z

@thomas-bc - I've pushed fixes for the cmake test & macOs build failures. Could you retrigger the workflow?

thomas-bc · 2026-05-28T16:28:02Z

@lestarch-autobot

LeStarch

This is in quite good shape! I confirmed that other major OSes have this, so it is reasonable in our OSAL and for use in components!

Minor changes needed. Will send the review bot too.

LeStarch · 2026-06-01T17:36:50Z

+
+namespace Os {
+
+CountingSemaphoreInterface* CountingSemaphoreInterface::getDelegate(CountingSemaphoreHandleStorage& aligned_new_memory,


Why a U32 here?

lestarch-autobot · 2026-06-01T18:02:13Z

+Os::CountingSemaphore::Status DarwinCountingSemaphore::post() {
+    FW_ASSERT(this->m_handle.m_semaphore != nullptr);
+    long result = dispatch_semaphore_signal(this->m_handle.m_semaphore);
+    FW_ASSERT(result <= 0);


must fix Inverted assert crashes when post() wakes a waiting thread.

dispatch_semaphore_signal returns 0 (no waiter woken) or a positive value (waiter woken). The predicate result <= 0 fails on the positive-return case, asserting under the semaphore's normal signaling path. This makes post() unusable whenever a thread is blocked in wait().

Finding-class: general-vulnerability/incorrect-assert

Suggested change

FW_ASSERT(result <= 0);

FW_ASSERT(result >= 0);

lestarch-autobot · 2026-06-01T18:02:13Z

+
+CountingSemaphoreInterface* CountingSemaphoreInterface::getDelegate(CountingSemaphoreHandleStorage& aligned_new_memory,
+                                                                    U32 initial_count) {
+    return new (aligned_new_memory) Os::Posix::Semaphore::PosixCountingSemaphore(initial_count);


must fix Raw placement-new bypasses Os::Delegate::makeDelegate size guard — potential buffer overflow.

makeDelegate enforces static_assert(sizeof(Impl) <= sizeof(Storage)). This getDelegate skips it. On Linux x86_64 sizeof(PosixCountingSemaphore) ≈ 40 (8-byte vtable + 32-byte sem_t) while FW_COUNTING_SEMAPHORE_HANDLE_MAX_SIZE = 32, overflowing m_handle_storage into the adjacent m_delegate field.
Add a manual static_assert or increase the handle size constant.

Finding-class: general-vulnerability/buffer-overflow

cc @LeStarch @thomas-bc @bitWarrior — low-confidence finding, please confirm.

Yes, we should use makeDelegate, or properly conform to its standards.

[Security] Disagreement — escalating. I still flag this on 9d4e06d; the contributor's response above indicates acknowledgment but the finding remains unfixed.

The raw placement new still bypasses Os::Delegate::makeDelegate and its static_assert(sizeof(Impl) <= sizeof(Storage)) guard. The contributor agreed the pattern should use makeDelegate, but the fix has not been applied.

cc @LeStarch @thomas-bc @bitWarrior — needs human adjudication.

lestarch-autobot

Automated review summary (run 1)

Per-agent results

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	2	0	0	0	2	No-Go
Supply Chain / Runner Safety	0	0	0	0	0	Go
F Prime C/C++ Design	3	6	0	0	9	No-Go
Documentation Currency	1	2	1	0	4	No-Go
Design	3	0	0	0	3	No-Go
Test Quality	3	1	0	0	4	No-Go
CI safety	—	—	—	—	—	Go
Totals	12	9	1	0	22	No-Go

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	clean
Review-system integrity	clean

Outstanding must-fix items (12)

Security Vulnerabilities

Inverted assert in Darwin post() crashes when waking a waiting thread — link
Raw placement-new bypasses Os::Delegate::makeDelegate size guard — potential buffer overflow — link

F Prime C/C++ Design

CPP-17: missing = delete on move ctor/assignment; copy-delete binds wrong type — link
CPP-19: dispatch_semaphore_t m_semaphore uninitialized — link
CPP-19: sem_t m_semaphore uninitialized — link

Documentation Currency

SDD section 5.2 API-usage table omits CountingSemaphore from Handles row — link

Design

Human design adjudication required. New OSAL core service introduces framework-level delegatable type — link
SDD describes "configurable max count" but no max_count parameter exists — link
waitTimeout(U32 timeout_ms) deviates from established OSAL timeout pattern (Fw::TimeInterval) — link

Test Quality

tryWait() has zero test coverage — link
PostWait / MultipleWaiters have no gtest assertions — rely on FW_ASSERT abort — link
Rule actions use FW_ASSERT instead of gtest assertions — kills process on failure — link

Merge readiness

Merge readiness: No-Go — 5 agent(s) have outstanding must-fix items: Security Vulnerabilities, F Prime C/C++ Design, Documentation Currency, Design, Test Quality

Agents that did not run on this PR

All registered reviewers ran successfully on this PR.

Solid foundation for a new OSAL primitive — twelve must-fix items to resolve before this module is cleared for flight.

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

+
+CountingSemaphore::Status CountingSemaphore::waitTimeout(const Fw::TimeInterval& interval) {
+    FW_ASSERT(&this->m_delegate == reinterpret_cast<CountingSemaphoreInterface*>(&this->m_handle_storage[0]));
+    return this->m_delegate.waitTimeout(interval);


+}
+
+StubCountingSemaphore::Status StubCountingSemaphore::waitTimeout(const Fw::TimeInterval& interval) {
+    (void)interval;


+namespace Semaphore {
+
+StubCountingSemaphore::StubCountingSemaphore(U32 initial_count) {
+    (void)initial_count;


+
+CountingSemaphore::Status posix_status_to_semaphore_status(int posix_status) {
+    CountingSemaphore::Status status = CountingSemaphore::Status::ERROR_OTHER;
+    switch (posix_status) {


+
+PosixCountingSemaphore::PosixCountingSemaphore(U32 initial_count) {
+    // Default all pthread semaphores to only be visible to single application (since F' FSW is a single application)
+    int status = sem_init(&this->m_handle.m_semaphore, 0, initial_count);


+    do {
+        status = sem_wait(&this->m_handle.m_semaphore);
+    } while (status != 0 && errno == EINTR);


+    int clock_status = clock_gettime(CLOCK_REALTIME, &abstime);
+    FW_ASSERT(clock_status == 0, static_cast<FwAssertArgType>(clock_status));
+
+    abstime.tv_sec += interval.getSeconds();


+    do {
+        status = sem_timedwait(&this->m_handle.m_semaphore, &abstime);
+    } while (status != 0 && errno == EINTR);


github-actions · 2026-06-04T21:28:31Z

Coverage report — base `devel`

Overall (line): 81.29% → 81.17% (-0.12)
Regression threshold: 0.50% (line).

Regressions

Module	Line	Δ	Function	Δ	Branch	Δ
`Os`	18.09	-0.73	19.52	-1.07	12.09	-0.61

Modules changed

Module	Line	Δ	Function	Δ	Branch	Δ
`Os`	18.09	-0.73	19.52	-1.07	12.09	-0.61
`Os/Posix`	61.90	+0.72	84.21	+0.49	40.61	+0.65

Modules without UTs

CFDP/Checksum/GTest, Drv/ByteStreamDriverModel, Drv/Interfaces, Drv/LinuxGpioDriver, Drv/LinuxI2cDriver, Drv/LinuxSpiDriver, Drv/LinuxUartDriver, Drv/Ports, Drv/Ports/DataTypes, FppTestProject/FppTest/interfaces, FppTestProject/FppTest/topology/async, FppTestProject/FppTest/topology/components/Comp, FppTestProject/FppTest/topology/components/Framework, FppTestProject/FppTest/topology/components/Receiver, FppTestProject/FppTest/topology/components/Sender, FppTestProject/FppTest/topology/guarded, FppTestProject/FppTest/topology/ports, FppTestProject/FppTest/topology/sync, FppTestProject/FppTest/topology/top_ports, FppTestProject/FppTest/topology/types, Fw/Cmd, Fw/Com, Fw/Comp, Fw/FilePacket/GTest, Fw/Fpy, Fw/Interfaces, Fw/Obj, Fw/Port, Fw/Ports/CompletionStatus, Fw/Ports/Ready, Fw/Ports/Signal, Fw/Ports/SuccessCondition, Fw/Prm, Fw/SerializableFile/test/TestSerializable, Fw/Sm, Fw/Test, Fw/Types/GTest, Os/Models, Svc/Cycle, Svc/DpPorts, Svc/Fatal, Svc/FatalHandler, Svc/FileDownlinkPorts, Svc/FprimeProtocol, Svc/Interfaces, Svc/PassiveConsoleTextLogger, Svc/Ping, Svc/PolyIf, Svc/Ports/CommsPorts, Svc/Ports/FilePorts, Svc/Ports/OsTimeEpoch, Svc/Ports/TlmPacketizerPorts, Svc/Ports/VersionPorts, Svc/Sched, Svc/Seq, Svc/Subtopologies/CdhCore, Svc/Subtopologies/ComCcsds, Svc/Subtopologies/ComFprime, Svc/Subtopologies/ComLoggerTee, Svc/Subtopologies/DataProducts, Svc/Subtopologies/FileHandling, Svc/Types/TlmPacketizerTypes, Svc/WatchDog, TestDeploymentsProject/Ref/PingReceiver, TestDeploymentsProject/Ref/RecvBuffApp, TestDeploymentsProject/Ref/SendBuffApp, TestDeploymentsProject/Ref/Top, TestDeploymentsProject/Ref/TypeDemo, Utils/Hash, cmake/test/data/TestDeployment/TestBuildAutocoder, cmake/test/data/TestDeployment/TestChainedAutocoder, cmake/test/data/TestDeployment/TestHeaderAutocoder, cmake/test/data/TestDeployment/TestTargetAutocoder, cmake/test/data/test-fprime-library/TestLibrary/TestComponent, cmake/test/data/test-fprime-library2/TestLibrary2/TestComponent

lestarch-autobot · 2026-06-04T21:33:09Z

+
+CountingSemaphoreInterface* CountingSemaphoreInterface::getDelegate(CountingSemaphoreHandleStorage& aligned_new_memory,
+                                                                    U32 initial_count) {
+    return new (aligned_new_memory) Os::Darwin::Semaphore::DarwinCountingSemaphore(initial_count);


[Security] could fix Raw placement new bypasses Os::Delegate::makeDelegate compile-time size guard.

Same pattern flagged on the Posix delegate. DarwinCountingSemaphore currently fits within FW_COUNTING_SEMAPHORE_HANDLE_MAX_SIZE (32), but there is no static_assert to catch future growth. Using makeDelegate<CountingSemaphoreInterface, DarwinCountingSemaphore> adds the guard for free.

Finding-class: general-vulnerability/buffer-overflow

cc @LeStarch @thomas-bc @bitWarrior — low-confidence finding, please confirm.

lestarch-autobot · 2026-06-04T21:34:25Z

+// ======================================================================
+#include "Os/Posix/CountingSemaphore.hpp"
+#include <errno.h>
+#include <time.h>


[C++ Design] suggestion CPP-26 (cpp-style-guide-violation): Use C++ headers <cerrno> and <ctime> instead of C headers <errno.h> and <time.h>.

Every other Posix OSAL source file in Os/Posix/ uses the C++ wrappers (<cerrno>, <ctime>). Aligning here keeps the include style consistent across the layer.

Suggested change

#include <time.h>

#include <cerrno>

#include <ctime>

lestarch-autobot · 2026-06-04T21:35:30Z

+    aggregator.notify(to_post);
+    aggregator.join();
+}


[Test Quality] must fix TimeoutSuccess test body has no ASSERT_* / EXPECT_* after aggregator.join().

The WaitTimeout rule action accepts both OP_OK and ERROR_TIMEOUT (line 40), so this test passes silently whether the post woke the waiter or the wait timed out. Every other concurrent test (PostWait, MultipleWaiters, FairnessVerification, ManyThreadsStress) asserts tester.waiters == 0 after join — this one should too.

Suggested change

aggregator.notify(to_post);

aggregator.join();

}

aggregator.notify(to_post);

aggregator.join();

ASSERT_EQ(tester.waiters, 0U) << "Waiter should have completed via post, not timeout";

}

lestarch-autobot · 2026-06-04T21:35:36Z

+
+void Tester::Wait::action(Tester& state) {
+    ++state.waiters;
+    ::Os::CountingSemaphore::Status status = state.semaphore.wait();


[Test Quality] must fix Wait::action() calls semaphore.wait() while the ConcurrentRule global lock is held (acquired in ConcurrentRule::run() via Os::ScopeLock lock(self->getLock())). If the semaphore count were ever 0, sem_wait would block with the global mutex still locked, deadlocking all other concurrent rules — no Post rule could ever acquire the lock to call post() and unblock the waiter.

The existing OSAL concurrent tests handle this correctly:

Queue (Os/test/ut/queue/QueueRules.cpp): explicitly calls getLock().unlock() before the blocking queue.send(…, BLOCKING) / queue.receive(…, BLOCKING), then getLock().lock() afterward.

ConditionVariable (Os/test/ut/condition/ConditionRules.cpp): calls state.m_condition.wait(this->getLock()) which atomically releases the lock.

This rule should unlock the global lock before calling semaphore.wait() and re-lock afterward, following the Queue pattern:

void Tester::Wait::action(Tester& state) { ++state.waiters; this->notify_other("Post"); // signal Post to proceed getLock().unlock(); ::Os::CountingSemaphore::Status status = state.semaphore.wait(); getLock().lock(); --state.waiters; ASSERT_EQ(status, ::Os::CountingSemaphore::Status::OP_OK); }

Without this fix, the concurrent test infrastructure cannot exercise the critical blocking path of wait(), which is the primary behavior a counting semaphore's concurrent tests should verify.

lestarch-autobot · 2026-06-04T21:35:36Z

+    Os::CountingSemaphore semaphore;
+    U32 waiters;
+
+    Tester() : semaphore(10U), waiters(0U) {}


[Test Quality] must fix The Tester initializes semaphore(10U), giving the semaphore an initial count of 10. Combined with the global-lock issue in Wait::action(), this means semaphore.wait() always returns immediately — the count never reaches 0, so no Wait thread ever blocks.

All "concurrent" tests (PostWait, MultipleWaiters, FairnessVerification, ManyThreadsStress) degenerate to sequential execution: each Wait thread acquires the global lock, decrements from 10, and returns. The Post / main-thread post() calls merely increment an already-positive count. No thread ever contends on the semaphore.

To actually test blocking behavior, initialize with semaphore(0U) so that wait() must block until a post() from another thread:

Suggested change

Tester() : semaphore(10U), waiters(0U) {}

Tester() : semaphore(0U), waiters(0U) {}

(This also requires fixing Wait::action() to release the global lock before calling wait() — see the companion finding on CountingSemaphoreRules.cpp.)

lestarch-autobot · 2026-06-04T21:35:36Z

+    Os::Test::CountingSemaphore::Tester::Post post_rule(aggregator);
+
+    aggregator.apply(tester);
+    // Brief wait to ensure Wait blocks on semaphore before triggering Post


[Test Quality] must fix This comment states "Brief wait to ensure Wait blocks on semaphore before triggering Post", but with semaphore(10U) (from RulesHeaders.hpp), the Wait rule's semaphore.wait() returns immediately — it never blocks. The 10ms delay and subsequent notify(to_post) are operating on a Wait thread that has already completed.

This test would pass even if wait() were a no-op that unconditionally returned OP_OK. The ASSERT_EQ(tester.waiters, 0U) at line 33 succeeds because all waiters already finished (count was 10→9), not because they were unblocked by post().

The same pattern affects MultipleWaiters (line 55), FairnessVerification (line 137), ManyThreadsStress (line 162), and TimeoutSuccess (line 104) — all rely on Task::delay() to "ensure" threads are blocked, but none ever actually block because the initial count is too high.

To make this test meaningful, the semaphore must start at 0 and Wait::action() must release the global lock before blocking (see companion findings). The test should then verify that the Wait thread is actually blocked (e.g. check tester.waiters > 0 before posting) rather than relying on a timing delay.

Compare with the ConditionVariable WaitAll test (Os/test/ut/condition/CommonTests.cpp) which correctly spins on tester.m_waiters != 2 under the lock to confirm threads are actually blocked before proceeding:

// Condition variable test — correct pattern: Os::ScopeLock lock(aggregator.getLock()); while (tester.m_waiters != 2) { pseudoRule.wait_for_next_step(); }

…maphore Test Quality (must-fix): - Tester: Initialize semaphore with count 0 instead of 10 so concurrent tests exercise actual blocking behavior rather than always returning immediately from a pre-filled count. - Wait::action / WaitTimeout::action: Release the global ConcurrentRule lock before calling the blocking semaphore wait, re-acquire after. Follows the Queue test pattern (QueueRules.cpp) to prevent deadlock where Post can never acquire the lock to unblock the waiter. - TimeoutSuccess test: Add ASSERT_EQ(tester.waiters, 0U) after join() to verify the waiter completed via post rather than timeout. C++ Design (suggestion): - Posix CountingSemaphore: Use C++ headers <cerrno>/<ctime> instead of C headers <errno.h>/<time.h> for consistency with other Os/Posix/ source files. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

AggregatedConcurrentRule::notify() asserts fatally when the named rule is not registered. Tests like MultipleWaiters, FairnessVerification, and ManyThreadsStress only register Wait rules without a Post rule, so the unconditional notify_other("Post") would crash those tests. The lock release/reacquire around the blocking call is the essential fix; the main thread already handles Post coordination via delay + direct semaphore.post() or aggregator.notify(). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Add a variadic-args overload of Os::Delegate::makeDelegate that forwards constructor arguments via perfect forwarding. This enables CountingSemaphore (which requires an initial_count parameter) to use the same compile-time-guarded delegate pattern as all other OSAL services instead of manually replicating static_asserts + placement new. Converted all four DefaultCountingSemaphore.cpp files (Posix, Darwin, Stub, Stub/test) to use the new overload. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

This reverts commit d361b02.

Remove all #ifdef __APPLE__ guards and #pragma suppression from Os/Posix/CountingSemaphore.cpp. Instead, wrap the register_os_implementation("CountingSemaphore" ...) call in if(NOT APPLE) in CMakeLists.txt so the Posix implementation is never compiled on macOS. The Darwin implementation provides full macOS coverage. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

- Add FW_COUNTING_SEMAPHORE_HANDLE_MAX_SIZE to default/config/PlatformCfg.fpp following the standard pattern used by all other OSAL handle sizes. - Add CountingSemaphoreHandleStorage typedef to Os/Os.hpp, consistent with MutexHandleStorage, TaskHandleStorage, etc. - Remove the hardcoded COUNTING_SEMAPHORE_HANDLE_MAX_SIZE constant and local typedef from Os/CountingSemaphore.hpp. - Fix test_recursion.cmake to use hardcoded Posix (not ${FPRIME_PLATFORM}) for CountingSemaphore dependencies, matching Console/File/Mutex/Task/RawTime which also use the Posix implementation directly on Linux. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

…xes' into count-sem

bevinduckett added 3 commits May 27, 2026 03:36

Added Os::CountingSemaphore osal module (nasa#5173)

e2636bd

Added CountingSemaphore to SDD

e7f0338

Added new words for spell check of semaphore

5cdb769

bevinduckett mentioned this pull request May 27, 2026

Message Queue with per priority memory allocation #5205

Draft

Merge branch 'devel' into count-sem

a4d838c

github-advanced-security AI found potential problems May 27, 2026

View reviewed changes

Comment thread Os/CountingSemaphore.cpp Fixed

Comment thread Os/CountingSemaphore.hpp Fixed

Comment thread Os/Posix/CountingSemaphore.cpp Fixed

Comment thread Os/Posix/error.cpp

return status;

}

CountingSemaphore::Status posix_status_to_semaphore_status(int posix_status) {

github-advanced-security AI found potential problems May 27, 2026

View reviewed changes

bevinduckett added 2 commits May 27, 2026 22:15

Added CountingSemaphore for Darwin

af53216

Added CountingSemaphore to cmake test

b338f73

bevinduckett added 2 commits May 27, 2026 23:12

Corrected status type in Darwin/CountingSemaphore

47d56af

Removed pthread argument from CountingSemaphore constructor

2292b19

LeStarch requested changes Jun 1, 2026

View reviewed changes

lestarch-autobot reviewed Jun 1, 2026

View reviewed changes

Comment thread Os/CountingSemaphore.hpp

Comment thread Os/test/ut/countingsemaphore/CommonTests.cpp

Comment thread Os/test/ut/countingsemaphore/CountingSemaphoreRules.cpp Outdated

Comment thread Os/test/ut/countingsemaphore/CommonTests.cpp Outdated

lestarch-autobot reviewed Jun 1, 2026

View reviewed changes

Comment thread Os/CountingSemaphore.hpp

Comment thread Os/docs/sdd.md Outdated

Comment thread Os/CountingSemaphore.hpp Outdated

lestarch-autobot reviewed Jun 1, 2026

View reviewed changes

Comment thread Os/docs/sdd.md Outdated

Comment thread Os/CountingSemaphore.hpp

Comment thread Os/CountingSemaphore.hpp

Comment thread Os/CountingSemaphore.hpp

lestarch-autobot reviewed Jun 1, 2026

View reviewed changes

lestarch-autobot suggested changes Jun 1, 2026

View reviewed changes

thomas-bc and others added 7 commits June 1, 2026 16:16

Merge remote-tracking branch 'nasa/devel' into count-sem

321ec6d

Update Os/test/ut/countingsemaphore/CountingSemaphoreRules.cpp

ea60e4f

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

Update Os/docs/sdd.md

925f4e6

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

Update Os/Stub/test/CountingSemaphore.hpp

bb2515c

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

Update Os/Posix/CountingSemaphore.hpp

81307d2

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

Update Os/Darwin/CountingSemaphore.hpp

d707892

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

Update Os/Stub/CountingSemaphore.hpp

f9b0692

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

github-advanced-security AI found potential problems Jun 4, 2026

View reviewed changes

lestarch-autobot reviewed Jun 4, 2026

View reviewed changes

devin-ai-integration Bot mentioned this pull request Jun 4, 2026

Address must-fix and high-priority review findings for Os::CountingSemaphore JPL-Devin/fprime#154

Closed

thomas-bc and others added 3 commits June 4, 2026 23:06

new batch of review stuff

d40cfb3

thomas-bc reviewed Jun 5, 2026

View reviewed changes

Comment thread Os/Posix/DefaultCountingSemaphore.cpp

Comment thread Os/Posix/CountingSemaphore.cpp Outdated

thomas-bc and others added 3 commits June 5, 2026 00:10

Revert "Use makeDelegate for CountingSemaphore delegate factories"

9670502

This reverts commit d361b02.

Remove ifedf APPLE guards

1a1b4c0

github-advanced-security AI found potential problems Jun 5, 2026

View reviewed changes

Comment thread Os/Posix/CountingSemaphore.cpp Dismissed

thomas-bc and others added 4 commits June 4, 2026 17:28

use FPRIME_PLATFORM in CMake UTs

6988a7f

fix CMake dependencies that cause crashes

294d1fc

formatting

ceb2716

devin-ai-integration Bot mentioned this pull request Jun 5, 2026

Move CountingSemaphore handle size to PlatformCfg.fpp and fix CMake test JPL-Devin/fprime#156

Open

thomas-bc added 3 commits June 4, 2026 18:25

Merge remote-tracking branch 'JPL-Devin/devin/1780622308-count-sem-fi…

c1697b7

…xes' into count-sem

Remove FPRIME_STUB_FALLBACK_ALLOWED

7a2cc5b

formatting

9f5497e


		namespace Os {

		CountingSemaphoreInterface* CountingSemaphoreInterface::getDelegate(CountingSemaphoreHandleStorage& aligned_new_memory,

	Tester() : semaphore(10U), waiters(0U) {}
	Tester() : semaphore(0U), waiters(0U) {}

Conversation

bevinduckett commented May 27, 2026

Change Description

Rationale

Testing/Review Recommendations

Future Work

AI Usage (see policy)

Uh oh!

bevinduckett commented May 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bevinduckett commented May 27, 2026

Uh oh!

thomas-bc commented May 28, 2026

Uh oh!

LeStarch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lestarch-autobot left a comment

Choose a reason for hiding this comment

Automated review summary (run 1)

Per-agent results

Merge readiness

Agents that did not run on this PR

Uh oh!

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

Uh oh!

Coverage report — base devel

Regressions

Modules changed

github-actions Bot commented Jun 4, 2026 •

edited

Loading

Coverage report — base `devel`