Implement timeout capability. Apply timeout to crypto response by AlexLanzano · Pull Request #278 · wolfSSL/wolfHSM

AlexLanzano · 2026-01-28T22:35:26Z

fixes #130

Timeout Functionality: Client Perspective

1. Configuration at Init Time

When creating a client, you provide a whTimeoutConfig specifying the timeout duration and an optional callback:

whTimeoutConfig timeoutCfg = {
    .timeoutUs = WH_SEC_TO_USEC(5),   /* 5-second timeout */
    .expiredCb = myTimeoutHandler,      /* optional callback on expiry */
    .cbCtx     = myAppContext,          /* context passed to callback */
};

whClientConfig clientCfg = {
    .comm              = &commConfig,
    .respTimeoutConfig = &timeoutCfg,   /* attach timeout config */
};

wh_Client_Init(&clientCtx, &clientCfg);

During wh_Client_Init (src/wh_client.c:84-89), the config is copied into an embedded whTimeoutCtx respTimeout[1] inside the client context via wh_Timeout_Init(). This stores the timeout duration and callback but doesn't start any timer yet.

If respTimeoutConfig is NULL, the timeout context is left zeroed and effectively disabled (a timeoutUs of 0 means "never expires").

2. What Happens During a Crypto Call

Before this PR, every crypto function in wh_client_crypto.c had this pattern after sending a request:

/* Old pattern -- infinite busy-wait */
do {
    ret = wh_Client_RecvResponse(ctx, &group, &action, &res_len, dataPtr);
} while (ret == WH_ERROR_NOTREADY);

If the server never responded, the client would spin forever.

The PR replaces all ~30 of these with a single helper _recvCryptoResponse() (src/wh_client_crypto.c:165-180):

static int _recvCryptoResponse(whClientContext* ctx,
                               uint16_t* group, uint16_t* action,
                               uint16_t* size, void *data)
{
    int ret;

#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT
    ret = wh_Client_RecvResponseTimeout(ctx, group, action, size, data,
                                        ctx->respTimeout);
#else
    do {
        ret = wh_Client_RecvResponse(ctx, group, action, size, data);
    } while (ret == WH_ERROR_NOTREADY);
#endif

    return ret;
}

When timeout is enabled, it delegates to wh_Client_RecvResponseTimeout. When disabled, the old infinite-loop behavior is preserved.

3. The Timeout Receive Loop

wh_Client_RecvResponseTimeout (src/wh_client.c:211-231) does this:

Starts the timer -- calls wh_Timeout_Start() which snapshots the current time via WH_GETTIME_US() into timeout->startUs.
Polls for a response -- calls wh_Client_RecvResponse() in a loop.
On each WH_ERROR_NOTREADY, checks wh_Timeout_Expired():
- Gets the current time via WH_GETTIME_US()
- Computes (now - startUs) >= timeoutUs
- If expired: invokes the expiredCb (if set), then returns WH_ERROR_TIMEOUT
- If not expired: loops again
On any other return value (success or error), returns immediately.

Client App                    _recvCryptoResponse                 wh_Timeout
    |                                |                                |
    |-- wh_Client_AesCbc() --------> |                                |
    |                                |-- wh_Timeout_Start --------> capture time
    |                                |                                |
    |                                |-- RecvResponse (NOTREADY)      |
    |                                |-- Expired? ------------------> no
    |                                |-- RecvResponse (NOTREADY)      |
    |                                |-- Expired? ------------------> no
    |                                |   ...                          |
    |                                |-- RecvResponse (NOTREADY)      |
    |                                |-- Expired? ------------------> YES
    |                                |                                |-- expiredCb()
    |<-- WH_ERROR_TIMEOUT -----------|                                |

4. What the Client Sees

From the application's perspective, the crypto APIs (wh_Client_AesCbc, wh_Client_RsaFunction, wh_Client_EccSign, etc.) now return WH_ERROR_TIMEOUT (-2010) instead of hanging indefinitely. The application can then decide how to handle it -- retry, log, fail gracefully, etc.

The expiredCb fires before the error is returned, so you can use it for logging or cleanup without needing to check the return code first.

5. Scope Limitations

A few things to note about the current design:

Only crypto responses are covered. Non-crypto client calls (key management, NVM operations, comm init) still use the old infinite-wait pattern. The timeout is specifically wired into _recvCryptoResponse.
The timeout is per-client, not per-call. All crypto operations for a given client share the same respTimeout context with the same duration. You can call wh_Timeout_Set(ctx->respTimeout, newValue) to change it between calls, but there's no per-operation override.

Copilot

Pull request overview

This PR introduces a generic timeout utility and wires client-side response timeouts into crypto operations, plus unit tests for the timeout helper.

Changes:

Add a generic timeout module (wh_timeout.[ch]) based on WH_GETTIME_US() and expose it via configuration (WOLFHSM_CFG_ENABLE_TIMEOUT) and a new error code WH_ERROR_TIMEOUT.
Extend whClientContext/whClientConfig and add wh_Client_RecvResponseTimeout, then route all crypto client receive paths through a new _recvCryptoResponse helper that uses the timeout when enabled.
Add unit tests for the timeout helper and enable timeout support in the test configuration.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
wolfhsm/wh_timeout.h	Declares timeout context/config types and the timeout API used by the client; documentation establishes that `timeoutUs == 0` disables the timeout.
wolfhsm/wh_settings.h	Documents `WOLFHSM_CFG_ENABLE_TIMEOUT` as enabling timeout helpers and client response timeouts; also defines `WH_GETTIME_US()`, which the timeout code relies on.
wolfhsm/wh_error.h	Introduces `WH_ERROR_TIMEOUT` to signal an expired timeout from client operations.
wolfhsm/wh_client.h	Adds a per-client `respTimeout` context, an optional `respTimeout` config, and declares `wh_Client_RecvResponseTimeout` behind `WOLFHSM_CFG_ENABLE_TIMEOUT`.
test/wh_test_timeout.h	Declares the `whTest_Timeout` unit test entry point.
test/wh_test_timeout.c	Implements unit tests for `wh_Timeout_*`, including callback invocation, stop/disable behavior, and bad-argument handling.
test/wh_test.c	Wires `whTest_Timeout()` into the unit test suite when `WOLFHSM_CFG_ENABLE_TIMEOUT` is defined.
test/config/wolfhsm_cfg.h	Enables `WOLFHSM_CFG_ENABLE_TIMEOUT` in the test configuration and ensures a valid time source via `WOLFHSM_CFG_PORT_GETTIME`.
src/wh_timeout.c	Implements the timeout helper functions; currently treats `timeoutUs == 0` as an error in `wh_Timeout_Start`, which conflicts with the documented “0 disables” semantics and impacts higher-level usage.
src/wh_client_crypto.c	Introduces `_recvCryptoResponse` and switches all crypto receive loops to use it; with timeout support enabled this always goes through `wh_Client_RecvResponseTimeout` and the per-client `respTimeout`.
src/wh_client.c	Initializes the optional `respTimeout` context in `wh_Client_Init` and adds `wh_Client_RecvResponseTimeout`, which loops on `WH_ERROR_NOTREADY` until a valid response arrives or the timeout expires.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/wh_client.c

src/wh_timeout.c

src/wh_client_crypto.c

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/wh_test_timeout.c

src/wh_client_crypto.c

src/wh_timeout.c

wolfhsm/wh_timeout.h

src/wh_client.c

bigbrett

Overall looks great. Some smaller tweaks and also proposed an extension of functionality that we might want.

wolfhsm/wh_client.h

wolfhsm/wh_settings.h

test/wh_test_timeout.h

wolfhsm/wh_client.h

src/wh_client.c

src/wh_timeout.c

test/wh_test_timeout.c

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/wh_comm.c

wolfhsm/wh_timeout.h

wolfhsm/wh_comm.h

docs/draft/timeout.md

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/wh_comm.c

port/posix/posix_timeout.c

test/Makefile

docs/draft/timeout.md

bigbrett

Some nits and some bigger architectural changes to testing now that we refactored timeouts to comm layer, therefore making it fully async

bigbrett · 2026-03-04T22:09:21Z

wolfhsm/wh_timeout.h

+    whTimeoutExpiredCb expiredCb;  /* Application expired callback */
+    void*              expiredCtx; /* Application callback context */
+    int                initialized;
+    uint8_t            WH_PAD[4];


We only need to pad wire-structs. This also is incorrect padding as int is only 4 bytes on 32-bit systems

bigbrett · 2026-03-04T22:12:31Z

wolfhsm/wh_comm.h


 #include <stdint.h>  /* For sized ints */

+#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT


shouldn't need to conditionally guard the include, only the struct fields

bigbrett · 2026-03-04T22:14:00Z

test/Makefile


 # Enable extra C compiler warnings
-CFLAGS_EXTRA = -Werror -Wall -Wextra
+CFLAGS_EXTRA = -Werror -Wall -Wextra -g


listen to the robot here - this was missed and should be removed

I think you were looking at an old commit. I removed this in my "Address comments from copilot" commit from yesterday

bigbrett · 2026-03-04T22:27:34Z

port/posix/posix_timeout.c

+    if (ctx->initialized) {
+        return WH_ERROR_OK;
+    }


Correct behavior is probably to re-initialize every time it is called, not just return ok. Otherwise state is not reset properly.

bigbrett · 2026-03-04T22:38:06Z

port/posix/posix_timeout.c

+    }
+
+    if (!ctx->initialized) {
+        return WH_ERROR_NOTREADY;


Recommend WH_ERROR_BADARGS. WH_ERROR_NOTREADY has a specific transport-focused meaning.

bigbrett · 2026-03-04T22:40:40Z

port/posix/posix_timeout.h

+typedef struct posixTimeoutContext_t {
+    uint64_t startUs;     /* Snapshot of start time */
+    uint64_t timeoutUs;   /* Configured timeout duration */
+    int      started;     /* 1 if timer is running, 0 otherwise */


micro nit: how about running like you commented, or active? started sounds weird. IDK.

bigbrett · 2026-03-04T22:41:50Z

test/wh_test_timeout.c

 #include "wolfhsm/wh_timeout.h"
 #include "wolfhsm/wh_error.h"

+#include "port/posix/posix_timeout.h"


no blind include of POSIX layer

bigbrett · 2026-03-04T22:42:25Z

test/wh_test_timeout.c

-        .timeoutUs = 1,
-        .expiredCb = NULL,
-        .cbCtx     = NULL,
+    posixTimeoutContext posixCtx                         = {0};


git-clang-format main

bigbrett · 2026-03-04T22:51:36Z

test/wh_test_timeout.c

-        .timeoutUs = 1,
-        .expiredCb = NULL,
-        .cbCtx     = NULL,
+    posixTimeoutContext posixCtx                         = {0};


this is just blindly using posix stuff unprotected by WOLFHSM_CFG_TEST_POSIX. Protect with WOLFHSM_CFG_TEST_POSIX and name the test accordingly such that it is obviously a posix-only sequential test for now. Same comment for other tests in this file.

Ideally we could make this test a generic sequential config test, since nothing about the test logic itself is POSIX. Could just take client and server configs as input args then init, run tests, cleanup. This would allow for the same test to be run with multiple back-ends to test other timeout implementations. Yet another argument for splitting the test implementations from the underlying harness.

In general, if your test includes setup/tear down for a specific backend inside, then it is probably incorrect and should be refactored to be generic and support running arbitrary configurations so it can be reused in other harnesses.

Crossed out stuff addressed with my other comment to this file about adding a client-only test as the primary test now that we can be async with timeouts

bigbrett · 2026-03-04T23:03:32Z

test/wh_test_timeout.c

 #endif /* !WOLFHSM_CFG_NO_CRYPTO */

-static int whTest_TimeoutCb(whTimeoutCtx* ctx, int* isExpired)
+static int whTest_TimeoutCb(whTimeout* timeout, int* isExpired)


Now that you refactored the timeouts to be part of the comm layer, and therefore inherently async, I'm not sure we really need this test to be sequential anymore. Couldn't it be client-only now, since we don't really need to step the server and even process the response anymore? We could just set a timeout on like an echo message and then poll it in a loop to ensure the timeout expires.

The "sequential" test is really only needed now to test that blocking crypto can be interrupted. This is still a good test to have IMO, but doesn't need to be the primary test here since it is likely POSIX only. This lets us still test timeouts on target against a backend supplied by the user (probably just have it be a client config or context test where the client config or context is passed in as an arg and we just run the test on that client context, with optional init/cleanup depending if it is a context test or a config test.

IMO this is super valuable coverage added and lets ports use the test to validate their own callback implementations

Co-authored-by: Brett Nicholas <7547222+bigbrett@users.noreply.github.com>

Change the whTimeoutExpiredCb return type from void to int so callbacks can signal errors. wh_Timeout_Expired now propagates any non-zero callback return value to the caller. Update tests to match the new signature and add WH_TEST_PRINT to timeout test functions. Document the expiration override mechanism in both the doxygen headers and the timeout draft docs.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/wh_test_timeout.c

port/posix/posix_timeout.c

wolfhsm/wh_timeout.h

docs/draft/timeout.md

test/wh_test_timeout.c

src/wh_comm.c

test/wh_test_timeout.c

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/wh_test_timeout.c

src/wh_comm.c

wolfhsm/wh_timeout.h

src/wh_comm.c

test/wh_test_timeout.c

AlexLanzano requested review from bigbrett, billphipps and Copilot January 28, 2026 22:35

AlexLanzano assigned bigbrett and billphipps Jan 28, 2026

Copilot started reviewing on behalf of AlexLanzano January 28, 2026 22:35 View session

AlexLanzano force-pushed the timeout branch from 7237d55 to e776f9f Compare January 28, 2026 22:39

Copilot AI reviewed Jan 28, 2026

View reviewed changes

src/wh_client.c Outdated Show resolved Hide resolved

src/wh_timeout.c Outdated Show resolved Hide resolved

src/wh_client_crypto.c Outdated Show resolved Hide resolved

AlexLanzano marked this pull request as draft January 28, 2026 22:44

AlexLanzano force-pushed the timeout branch 2 times, most recently from 5498633 to f7a30b3 Compare January 30, 2026 18:27

bigbrett assigned AlexLanzano and unassigned bigbrett and billphipps Jan 30, 2026

AlexLanzano force-pushed the timeout branch from f7a30b3 to 26d6d08 Compare January 30, 2026 19:42

AlexLanzano marked this pull request as ready for review January 30, 2026 19:45

AlexLanzano requested a review from Copilot January 30, 2026 19:46

AlexLanzano assigned bigbrett and billphipps and unassigned AlexLanzano Jan 30, 2026

Copilot started reviewing on behalf of AlexLanzano January 30, 2026 19:46 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

AlexLanzano added a commit to AlexLanzano/wolfHSM that referenced this pull request Jan 30, 2026

Address comments on wolfSSL#278

b53143c

AlexLanzano added a commit to AlexLanzano/wolfHSM that referenced this pull request Jan 30, 2026

Address comments on wolfSSL#278

c367257

AlexLanzano force-pushed the timeout branch from b53143c to c367257 Compare January 30, 2026 20:32

bigbrett requested changes Feb 4, 2026

View reviewed changes

bigbrett assigned AlexLanzano and unassigned bigbrett Feb 4, 2026

AlexLanzano added a commit to AlexLanzano/wolfHSM that referenced this pull request Feb 5, 2026

Address comments on wolfSSL#278

390bd2a

AlexLanzano added a commit to AlexLanzano/wolfHSM that referenced this pull request Feb 5, 2026

Address comments on wolfSSL#278

9e3e50d

AlexLanzano dismissed bigbrett’s stale review via 9656bae March 3, 2026 15:57

Copilot started reviewing on behalf of AlexLanzano March 3, 2026 15:58 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

src/wh_comm.c Outdated Show resolved Hide resolved

wolfhsm/wh_timeout.h Outdated Show resolved Hide resolved

wolfhsm/wh_comm.h Show resolved Hide resolved

docs/draft/timeout.md Show resolved Hide resolved

docs/draft/timeout.md Show resolved Hide resolved

AlexLanzano requested a review from Copilot March 3, 2026 16:13

Copilot started reviewing on behalf of AlexLanzano March 3, 2026 16:13 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

src/wh_comm.c Show resolved Hide resolved

port/posix/posix_timeout.c Outdated Show resolved Hide resolved

port/posix/posix_timeout.c Outdated Show resolved Hide resolved

test/Makefile Outdated Show resolved Hide resolved

docs/draft/timeout.md Show resolved Hide resolved

AlexLanzano requested review from bigbrett and rizlik March 3, 2026 16:49

AlexLanzano assigned bigbrett Mar 3, 2026

bigbrett requested changes Mar 4, 2026

View reviewed changes

AlexLanzano and others added 11 commits March 6, 2026 13:54

Implement timeout capability. Apply timeout to crypto response

c754227

Address comments on wolfSSL#278

92658d3

Add draft documentation for timeout feature

d8a6fb2

Update wolfhsm/wh_settings.h

2b09e60

Co-authored-by: Brett Nicholas <7547222+bigbrett@users.noreply.github.com>

Address comments on wolfSSL#278

7d3fd5d

git-clang-format changes

2a1c03e

Address comments

a7a7e1d

Refactor timeout feature to use callbacks

af61b3c

Move response timeout logic into comm layer

2a3c6d3

Address comments from copilot

5a07940

Copilot AI review requested due to automatic review settings March 6, 2026 21:37

AlexLanzano force-pushed the timeout branch from 5468b13 to 75f918b Compare March 6, 2026 21:37

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Copilot started reviewing on behalf of AlexLanzano March 6, 2026 21:46 View session

AlexLanzano force-pushed the timeout branch from 75f918b to bc10fd9 Compare March 6, 2026 22:07

Copilot AI review requested due to automatic review settings March 6, 2026 22:44

Address Brett's comments

8472a87

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Address copilot comments

000d224


		#include <stdint.h> /* For sized ints */

		#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT

Conversation

AlexLanzano commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Timeout Functionality: Client Perspective

1. Configuration at Init Time

2. What Happens During a Crypto Call

3. The Timeout Receive Loop

4. What the Client Sees

5. Scope Limitations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bigbrett left a comment

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bigbrett left a comment

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!

bigbrett Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

AlexLanzano commented Jan 28, 2026 •

edited

Loading

bigbrett Mar 4, 2026 •

edited

Loading