esp_websocket_client: Add Kconfig options for PSRAM allocation (IDFGH-17011)#980
esp_websocket_client: Add Kconfig options for PSRAM allocation (IDFGH-17011)#980shardt68 wants to merge 1 commit into
Conversation
13277dd to
27c4e4b
Compare
27c4e4b to
8a75888
Compare
8a75888 to
b683518
Compare
b683518 to
6c381b9
Compare
6c381b9 to
e84c0eb
Compare
cbf6bad to
016bf62
Compare
016bf62 to
0ae9008
Compare
15cba04 to
9d9f1c9
Compare
9d9f1c9 to
391fb0b
Compare
|
Hi @gabsuren, I wanted to follow up on this PR to see if there is any feedback or if any additional information is needed from my side to move this forward. These changes are quite important for memory-constrained applications using multiple concurrent TLS/HTTPS connections (like Spotify Connect implementations). By allowing the WebSocket task stack and client structure to be moved to PSRAM, we can significantly free up critical internal RAM. Beyond the memory allocation, this PR also introduces a more robust lifecycle management (using DESTRUCTION_IN_PROGRESS_BIT and deferred task deletion). This hardening has proven to be very stable and effectively prevents race conditions during rapid reconnect/destroy cycles that were previously an issue. The branch is currently conflict-free and has been extensively tested on ESP32-S3. Looking forward to your thoughts! |
gabsuren
left a comment
There was a problem hiding this comment.
Left some notes. Please take a look.
The task cleanup logic is duplicated across 4 different locations. This increases maintenance burden and the risk that future changes will introduce bugs if not applied consistently to all 4 spots.
|
@shardt68 Regarding the concurrency issues you encountered: Thank you again! |
|
Hi @gabsuren, Thank you for the guidance! I have updated the PR to focus strictly on the PSRAM allocation configuration. Specifically, I have: Reverted the version bump in idf_component.yml and CHANGELOG.md.
Example Scenario:
I have already prepared these hardening fixes in a separate branch and will submit them as a new PR for independent review once this PSRAM feature is addressed. Please let me know if this version looks good for merging! Best regards, |
d2242b7 to
61a8100
Compare
|
@shardt68 thank you for simplifying the PR to consider only the PSRAM part. Best regards, |
@shardt68 sorry I missed your last message, thanks for providing an example. The deadlock occurs because Instead of destroying the client immediately, signal it to destroy itself upon exit. ❌ Incorrect (Causes Deadlock): ✅ Correct: Best regards, |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Hi @shardt68, thanks for iterating on this, the strict-allocation direction is good. A few things to address before merge
- Commits
Please rebase onto current master and squash the 7 commits into a single conventional commit, for example:
feat(websocket): add Kconfig options for PSRAM allocation
- Strict allocation is not fully strict yet
In esp_websocket_client_start(), the else { xTaskCreatePinnedToCore(...) } branch silently falls back to internal RAM when task_stack <= 0.
Since the user explicitly enabled PSRAM task-stack allocation, I think the behavior should be:
PSRAM stack allocation succeeds, or start fails with a clear error.
Please remove the fallback branch so the option consistently means “allocate the task stack in PSRAM, or fail”.
- Kconfig documentation / dependencies
Please document and enforce the expected dependencies:
- depends on SPIRAM
- depends on FREERTOS_SUPPORT_STATIC_ALLOCATION for the task-stack option
- mention that only the task stack moves to PSRAM; the TCB stays in internal RAM
- mention the allocation-failure behavior: return NULL / ESP_FAIL, no fallback to internal RAM
Looks good otherwise, once these are in, happy to take another pass. Thanks!
09ec076 to
2c3347f
Compare
|
@shardt68 thank you for the updates. Please fix the pre-commit check and address the minor notes I left. Other than that, I think the PR is mostly ready for merging. There is one remaining major issue with This was not an issue before because the task stack/TCB were owned by FreeRTOS. With the new external-stack path, they are client-owned buffers, so freeing them from the running task can lead to use-after-free or memory corruption. I think this should be fixed before enabling |
|
I addressed the remaining Bugbot findings. PSRAM allocations freed with free Please take another look. |
2c71d70 to
7e14cb1
Compare
f2c4c09 to
6e9a74f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6e9a74f. Configure here.
73b36ba to
6423484
Compare

Description
This PR introduces two new Kconfig options to the
esp_websocket_clientcomponent to allow memory allocation in external RAM (PSRAM):ESP_WS_CLIENT_TASK_STACK_IN_EXT_RAM: Enables allocation of the WebSocket task stack in PSRAM usingxTaskCreateStaticPinnedToCore.ESP_WS_CLIENT_ALLOC_IN_EXT_RAM: Enables allocation of theesp_websocket_clientstructure and its internal configuration storage in PSRAM.Motivation
In applications requiring multiple concurrent secure connections (e.g., TLS/HTTPS), internal RAM is a critical bottleneck. Each TLS session typically requires 16-20KB of internal RAM for buffers. By allowing the WebSocket client's task stack and internal structures to be moved to PSRAM, significant internal memory is freed for these critical network operations, improving overall system stability and preventing
ESP_ERR_NO_MEMfailures in memory-constrained scenarios.Implementation Details
heap_caps_callocwithMALLOC_CAP_SPIRAMfor the client structure and config.xTaskCreateStaticPinnedToCorefor the task stack when PSRAM is selected.Related
Testing
heap_caps_get_free_sizethat internal RAM usage decreased by the expected amount (~5KB for structure + stack size).Note
Medium Risk
Changes FreeRTOS task teardown and allocation paths; incorrect ordering could cause use-after-free on the PSRAM stack, though the suspend/delete design targets that. PSRAM allocation failures fail hard (no fallback), which differs from the PR description’s claimed fallback behavior.
Overview
Adds two SPIRAM-gated Kconfig options so memory-heavy WebSocket client pieces can live in PSRAM instead of internal RAM, with defaults unchanged.
ESP_WS_CLIENT_ALLOC_IN_EXT_RAMallocates the client object andwebsocket_config_storage_tviaheap_caps_calloc(..., MALLOC_CAP_SPIRAM)and frees them withheap_caps_free; failed allocation makesesp_websocket_client_init()return NULL (no internal-RAM fallback).ESP_WS_CLIENT_TASK_STACK_IN_EXT_RAM(requires static task allocation) creates the worker withxTaskCreateStaticPinnedToCore: stack in PSRAM, TCB in internal RAM. On normal stop the taskvTaskSuspends instead of self-deleting so teardown canvTaskDeletefrom another context before freeing the PSRAM stack;destroy_and_free_resourcesandesp_websocket_client_startafterstop()implement that lifecycle.esp_websocket_client_destroy_on_exit()returnsESP_ERR_NOT_SUPPORTEDwhen the PSRAM stack option is enabled.Reviewed by Cursor Bugbot for commit 6423484. Bugbot is set up for automated code reviews on this repo. Configure here.