Fix libgpiod V1 edge events on 32-bit platforms with 64-bit time_t

[wietsejorissen]

Fixes #2604

On a 32-bit platform whose libgpiod is built with _TIME_BITS=64, the V1 driver passes an 8-byte
struct timespec where the library expects 16, and reads struct gpiod_line_event into a 12-byte
managed struct where the native one is 24. TimeSpec models tv_sec with NativeLong (= IntPtr),
but tv_sec is a time_t, and _TIME_BITS=64 decouples the two: on armv7 long is 4 bytes and
time_t is 8.

Three consequences:

• the 50 ms edge-wait timeout never fires, so the detection loop never re-checks cancellation and
  Dispose() on a pin with a registered callback never returns;
• event_type is read at offset 8 instead of 16, so a Rising-only callback never fires;
• gpiod_line_event_read writes past the end of the managed struct on every event.

Change

Modelling struct timespec per platform would require knowing how the native library was compiled,
which is not observable from managed code. This stops passing one instead: take the line's event
descriptor with gpiod_line_event_get_fd, poll() it alongside a self-pipe used for cancellation,
and read the kernel's fixed-width gpioevent_data record directly. poll() takes an int timeout,
unlike ppoll(), so nothing on this path depends on _TIME_BITS. Field offsets in gpioevent_data
are architecture-independent; only total size varies with trailing alignment, so a whole record of
either 12 or 16 bytes is accepted.

gpiod_line_event_get_fd exists since libgpiod 1.0.1. Where it is absent the existing
gpiod_line_event_wait loop is kept, but only on 64-bit, where sizeof(long) == sizeof(time_t)
holds. A 32-bit process without the event descriptor gets PlatformNotSupportedException naming the
missing-symbol requirement, rather than reading structures whose size cannot be established.

No public API change. The V2 driver is untouched.

Behaviour differences

• Cancellation is immediate instead of waiting for the next timeout.
• The 20 Hz wakeup per subscribed line is gone. A 1 s backstop poll() timeout remains to bound
  disposal if a wake write is ever lost; it does no work when it expires.
• Cost per subscribed line grows by one pipe (2 fds). The event fd is owned by libgpiod, and the
  detection thread already existed.

Measurements

ConnectCore MP157 (armv7l, glibc 2.39, libgpiod 1.6.4 built with 64-bit time_t), Digi Embedded
Yocto 5.0:

	before	after
Dispose() on a pin with a callback	never returns, needs SIGKILL	returns in < 100 ms
edge classification, 34 button edges	rising=0 falling=34	correct, over 250+ edges
1000 register/unregister cycles, one line	??????	fd and thread counts flat from cycle 250 on

Reproduces identically on .NET 8 and .NET 9.0.18; IntPtr.Size is 4 in both, so the runtime's own
time64 work does not reach this.

Tests

LibGpiodV1DriverTests gains a hardware-gated test asserting that Dispose() returns when a pin has
a registered callback and no edge arrives.

Unit tests for the record parsing that need no hardware are written but not included: they require
InternalsVisibleTo on the shipping library, which is strong-name signed, and adding a friend
reference also invalidates 11 existing protected overrides in MockableGpioDriver (CS0507).
TryClassifyEventRecord is left as an internal seam, so they are a small addition if you can say
which surface you would prefer.

build.cmd -configuration Release on both System.Device.Gpio and System.Device.Gpio.Tests
reports 0 warnings, 0 errors.

Not covered

Tested only on 32-bit Arm with 64-bit time_t. No x64, arm64 or bookworm-armhf hardware available
here, so those paths are unverified by me. The new path avoids time_t by construction, but that is
an argument, not a test result. Hardware availability here is temporary, so follow-up needing the
board may have to be picked up elsewhere.

Two follow-ups, described rather than filed, since neither is a regression and both look like yours
to scope / say if you want them as issues:

• Dispose() called from inside a pin-value-changed callback deadlocks by joining its own detection
  task. Longstanding and platform-independent, but it sits in the code this PR reworks, so it would
  be wrong to call this change deadlock-safe while it stands. Repro and a suggested direction (skip
  the join when already on the detection task) available.
• The kernel can return up to 16 records per wake; this reads one at a time, as the current code
  does, so behaviour is unchanged. Batching needs stride detection to separate the 12- and 16-byte
  layouts mid-buffer.

Microsoft Reviewers: Open in CodeFlow

[wietsejorissen]

@dotnet-policy-service agree company="Calculus"

[krwq]

AI GENERATED — evidence-based review of PR #2605

Summary

The diagnosis in the PR body is verified: on 32-bit ARM with _TIME_BITS=64, TimeSpec (NativeLong TvSec; NativeLong TvNsec; where NativeLong = IntPtr) is 8 bytes managed vs. 16 bytes native, and GpioLineEvent is 12 bytes managed vs. 24 bytes native. This trivially explains the three observed symptoms (never-firing 50 ms timeout, event_type read at offset 8 instead of 16, and 12-byte write into a struct the kernel writes 24 bytes into). The V2 driver has no timespec dependency and is correctly left alone.

The chosen fix — gpiod_line_event_get_fd + poll() (int timeout) + reading the kernel's struct gpioevent_data directly — sidesteps _TIME_BITS by construction. Field offsets in gpioevent_data (__u64 timestamp at 0, __u32 id at 8) are ABI-invariant; only the trailing alignment varies. Accepting both 12- and 16-byte record lengths is correct. pollfd layout, nfds_t sizing (pointer-width nuint), and the existing Interop.close/read/write signatures all line up. The self-pipe / Interlocked.Exchange ownership guard for Dispose fd close, the 1 s poll backstop, and the constructor rollback (ReleaseLock + pipe close) are all correct. The regression test is a real regression test — it asserts Dispose returns within 10 s with no edge.

Recommendation: approve with one nit.

Non-blocking findings

1. libgpiod 1.0.1 claim is factually incorrect

Files:

• src/System.Device.Gpio/System/Device/Gpio/ExceptionHelper.cs:94 (user-visible message)
• src/System.Device.Gpio/System/Device/Gpio/Drivers/LibGpiodDriverEventHandler.cs:34-36 (comment)
• src/System.Device.Gpio/Interop/Unix/libgpiod/V1/Interop.libgpiod.cs:142-144 (XML doc)
• PR description

The claim that gpiod_line_event_get_fd is "available since libgpiod 1.0.1" is not correct. That symbol has been in the public header since at least v0.3 (Sep 2017, commit 500c3093 already refines an existing function), well before v1.0.0 was tagged in Feb 2018. It is present in v0.1, v0.2, v0.3.3, v1.0, and v1.0.1 (all four verified against git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git).

In practice this makes the s_lineEventFdUnavailable fallback essentially unreachable, which is fine. But the user-facing PlatformNotSupportedException message shipped in ExceptionHelper.cs will tell a 32-bit user with genuinely-old libgpiod that they need "libgpiod 1.0.1 or later," which is wrong and unhelpful for diagnosis.

Suggested fix: either drop the specific version and refer only to the symbol ("...requires a libgpiod build that exports gpiod_line_event_get_fd...") or use the correct earliest version. Same edit in the code comment, XML doc, and PR description.

Severity: warning. Confidence: High — verified against tagged headers in the upstream repo.

Verified OK

• Dispose double-close race: _disposeOwned via Interlocked.Exchange guarantees exactly one caller closes the pipe fds, and both callers await the detection task before returning, so a containing driver cannot release the line handle while the task is still polling.
• Constructor partial-failure rollback: pipeCreated gates fd close, and safeLineHandle.ReleaseLock() unwinds the earlier SubscribeForEvent. SubscribeForEvent runs before the try but has nothing to roll back if it itself throws.
• POLLERR/POLLHUP/POLLNVAL on the line fd throwing rather than continuing quietly is a deliberate, defensible behaviour change over the legacy path; the line is requested for the handler's lifetime so an unusable fd is a real lifetime error.
• V2 driver untouched is the right call — no timespec on that path.

Verdict block

Review: 1 finding. Blocking: 0. Non-blocking: 1. Questions: 0.
Investigated and dismissed: several (constructor teardown ordering, _disposeOwned race window, kernel gpioevent_data size logic, nfds_t/nuint match, POLLHUP semantics, comment about "reading process" governing record size — technically the kernel's ABI governs, not the reader's, but the code handles both sizes so drop).
Verified OK notes: 4.
Recommendation: approve with nits.

[krwq]

[Triage] Please verify this and fix - see AI review

[wietsejorissen]

Verified against upstream history: gpiod_line_event_get_fd predates 1.0.1. I removed the version-specific claim from the code and PR/issue descriptions, and now describe support in terms of the loaded libgpiod build exporting the symbol. I also renamed the exception resource accordingly.

[krwq]

here also

[wietsejorissen]

Updated here as well; the remarks now describe why the descriptor is used without claiming a minimum version.

[krwq]

here also

[wietsejorissen]

Updated here as well; the cache is now described in terms of whether the loaded build exports the symbol.

[krwq]

here also

[wietsejorissen]

Updated here as well; the fallback explanation now refers to descriptor availability and only uses the legacy layout where it is known safe.

[krwq]

here also

[wietsejorissen]

Updated here as well; this now describes a missing export rather than a pre-1.0.1 library.

[krwq]

here also

[wietsejorissen]

Updated here as well; this now describes builds without the export and the 64-bit restriction.

[krwq]

how is this related to 32/64-bitness?

[krwq]

please isolate the change, if this is some issue you're seeing please send separate PR

[wietsejorissen]

Agreed. I restored Task.Run so this PR preserves the existing thread model and remains scoped to the time64 fix. I have not observed a separate thread-pool starvation issue, so I am not opening a follow-up PR at this point.

[krwq]

AI GENERATED — follow-up: compatibility risk and scope

Could this break existing users?

Going through the concrete behaviour deltas on the paths that were already working (64-bit, and 32-bit systems where libgpiod is not built with _TIME_BITS=64):

Low / no risk

• V2 driver: untouched.
• Public API: unchanged.
• 32-bit with old-style time_t: the code now takes the new poll() path via gpiod_line_event_get_fd; that path is time_t-independent by construction, so it is at worst equivalent.
• 64-bit: same story — gpiod_line_event_get_fd is present and the fast path is taken.
• Fallback path on 32-bit without gpiod_line_event_get_fd: now throws PlatformNotSupportedException instead of silently corrupting reads. Given gpiod_line_event_get_fd has been in the public header since v0.1, this is effectively unreachable in practice, so essentially nobody who was previously "working" ends up here.

Behaviour changes worth calling out on the changelog

1. Errors on the line fd are now surfaced. POLLERR/POLLHUP/POLLNVAL throw IOException from the detection task. Previously the wait loop would have kept spinning or returned an error status; the new path is stricter. Any user who was tolerating a broken line silently will now see an exception on that pin's detection thread. Defensible, but a behaviour change.
2. Detection thread model changed — see the scope note below. This is a per-subscribed-pin change from a pooled worker to a dedicated OS thread.
3. Constructor can now throw IOException on pipe2 failure (EMFILE / ENFILE — process ran out of fds). Previously subscription could not fail on host resource exhaustion this way.
4. Dispose behaviour — a concurrent second Dispose() on the same handler used to be effectively a no-op re: descriptors; now both callers wait for the detection task before returning. Any code that was double-disposing and expecting the second call to return immediately will block for up to the poll backstop (1 s) or until the task exits. Unlikely to matter in practice.

Net: I do not see a realistic scenario where a previously-working configuration regresses, but the "line fd errors now throw" behaviour is worth a line in the release notes.

Relevance of the diff to the PR title

Title is "Fix libgpiod V1 edge events on 32-bit platforms with 64-bit time_t." Most of the diff is a direct consequence of the fix approach and belongs:

• pipe2/poll interop, gpiod_line_event_get_fd P/Invoke, TryClassifyEventRecord, the new event detection loop — the fix.
• Self-pipe wake, _disposeOwned interlocked guard, constructor pipe rollback, WakeDetectionTask — required because Dispose() now has to unblock a poll() on a real fd and safely close pipe descriptors. These are fix-necessary, not drive-by.
• Legacy path (InitializeLegacyEventDetectionTask) split out — needed so the fallback keeps compiling. Fine.

Scope creep candidate:

• Switch from Task.Run to Task.Factory.StartNew(..., TaskCreationOptions.LongRunning | TaskCreationOptions.DenyChildAttach, TaskScheduler.Default) in InitializeEventDetectionTask (LibGpiodDriverEventHandler.cs:161, 243-245). The time_t fix does not require this. The pre-existing legacy path was already Task.Run, so it was already parking a thread-pool thread per subscribed line — that is a separate, longstanding problem. Making the new path LongRunning is a good change on its merits (avoids pool starvation on users who subscribe to many lines), but it is not part of this PR's stated scope, it changes the thread model users observe (dedicated OS thread per subscribed pin), and it is easy to miss during review because it is bundled with the fix.

Suggested action: either (a) call this out explicitly in the PR description as an intentional improvement enabled by the rewrite, or (b) drop it from this PR and file it as a separate change that can also be applied to the legacy path for consistency. Not blocking either way, but the current PR body does not mention it.

Everything else in the diff traces back to the time_t fix.