Skip to content

Fixes and improvements in native socket - #3289

Merged
josesimoes merged 1 commit into
nanoframework:mainfrom
josesimoes:fix-socket
Apr 7, 2026
Merged

josesimoes merged 1 commit into
nanoframework:mainfrom
josesimoes:fix-socket

Conversation

@josesimoes

@josesimoes josesimoes commented Apr 7, 2026 •

Copy link
Copy Markdown
Member

Description

  • Fix mapping SOCK_ENOTCONN error.
  • Add wait events call on send/receive error to prevent race with lwIP stack.
  • Adjust TCP_SYNMAXRTX lwIP config to get ~30seconds timeout on connect.
  • Update declaration of Sys.Net assembly.

Motivation and Context

How Has This Been Tested?

  • Running MQTT test broker provided in the issue.

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved socket operation responsiveness by preventing tight retry loops during non-blocking operations
    • Fixed socket error code mapping for non-connected state scenarios
  • Configuration

    • Standardized TCP SYN retransmission retry limits across all supported platforms (ESP32 variants, ChibiOS, FreeRTOS) for more consistent connection behavior

- Fix mapping SOCK_ENOTCONN error.
- Add wait events call on send/receive error to prevent race with lwIP stack.
- Adjust TCP_SYNMAXRTX lwIP config to get ~30seconds timeout on connect.
- Update declaration of Sys.Net assembly.
@nfbot nfbot added Type: bug Type: enhancement Type: dependencies Pull requests that update a dependency file(s) or version labels Apr 7, 2026
@coderabbitai

coderabbitai Bot commented Apr 7, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

This PR updates the System.Net native interface by adjusting method lookup table indexing and field constants, adds scheduler-aware error handling for non-blocking socket operations, corrects an error code mapping, and standardizes TCP SYN retransmission configuration across multiple platform targets by introducing the TCP_SYNMAXRTX parameter.

Changes

Cohort / File(s) Summary
System.Net Native Interface
src/DeviceInterfaces/System.Net/sys_net_native.cpp, src/DeviceInterfaces/System.Net/sys_net_native.h
Updated method lookup table with additional NULL entry, adjusted assembly hash, incremented method metadata tuple, and reassigned Socket field index constants (removed FIELD___nonBlockingConnectInProgress and FIELD___nonBlockingConnectRightEndPoint; shifted FIELD___rightEndPoint from 9 to 7).
Socket Error Handling
src/DeviceInterfaces/System.Net/sys_net_native_System_Net_Sockets_NativeSocket.cpp, src/PAL/Lwip/lwIP_Sockets.cpp
Added WaitEvents scheduler yield in SendRecvHelper on EWOULDBLOCK error to prevent tight retry loops; corrected ENOTCONN error mapping from SOCK_EISCONN to SOCK_ENOTCONN.
LwIP TCP Retransmission Configuration
targets/ChibiOS/_include/lwipopts.h, targets/FreeRTOS/_common/include/lwipopts.h
Added TCP_SYNMAXRTX macro definition (defaults to 4) for compile-time TCP SYN retransmission limit control.
ESP32 Platform Configurations
targets/ESP32/_IDF/sdkconfig.default*
Added CONFIG_LWIP_TCP_SYNMAXRTX=4 setting to 18 ESP32 variant sdkconfig files, standardizing TCP SYN retransmission behavior across all ESP32 targets.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fixes and improvements in native socket' accurately summarizes the main change: addressing socket-related fixes and improvements across the native socket layer and configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@josesimoes josesimoes added the Area: Common libs Everything related with common libraries label Apr 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@targets/ESP32/_IDF/sdkconfig.default`:
- Line 58: The TCP SYN retransmission count CONFIG_LWIP_TCP_SYNMAXRTX is set to
4 but does not reach the intended ~30s connect timeout; update
CONFIG_LWIP_TCP_SYNMAXRTX from 4 to 5 so that, given
CONFIG_LWIP_TCP_RTO_TIME=1500 ms, the SYN retransmission schedule yields ~31.5s
total timeout and meets the target.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b3e21377-abbb-4c63-b457-15673a0046c2

📥 Commits

Reviewing files that changed from the base of the PR and between be1bb6f and 1825493.

📒 Files selected for processing (25)
  • src/DeviceInterfaces/System.Net/sys_net_native.cpp
  • src/DeviceInterfaces/System.Net/sys_net_native.h
  • src/DeviceInterfaces/System.Net/sys_net_native_System_Net_Sockets_NativeSocket.cpp
  • src/PAL/Lwip/lwIP_Sockets.cpp
  • targets/ChibiOS/_include/lwipopts.h
  • targets/ESP32/_IDF/sdkconfig.default
  • targets/ESP32/_IDF/sdkconfig.default.esp32c3
  • targets/ESP32/_IDF/sdkconfig.default.esp32c5
  • targets/ESP32/_IDF/sdkconfig.default.esp32c6
  • targets/ESP32/_IDF/sdkconfig.default.esp32h2
  • targets/ESP32/_IDF/sdkconfig.default.esp32p4
  • targets/ESP32/_IDF/sdkconfig.default.esp32s2
  • targets/ESP32/_IDF/sdkconfig.default.esp32s3
  • targets/ESP32/_IDF/sdkconfig.default_ble.esp32s3
  • targets/ESP32/_IDF/sdkconfig.default_ble_rev3.esp32
  • targets/ESP32/_IDF/sdkconfig.default_nopsram.esp32
  • targets/ESP32/_IDF/sdkconfig.default_nopsram_ble.esp32
  • targets/ESP32/_IDF/sdkconfig.default_nopsram_rev3.esp32
  • targets/ESP32/_IDF/sdkconfig.default_octal_ble.esp32s3
  • targets/ESP32/_IDF/sdkconfig.default_pico
  • targets/ESP32/_IDF/sdkconfig.default_rev3.esp32
  • targets/ESP32/_IDF/sdkconfig.default_rev3.esp32c3
  • targets/ESP32/_IDF/sdkconfig.default_rev3_ipv6.esp32
  • targets/ESP32/_IDF/sdkconfig.default_rev3_noconsole.esp32c3
  • targets/FreeRTOS/_common/include/lwipopts.h

Comment thread targets/ESP32/_IDF/sdkconfig.default
@josesimoes
josesimoes merged commit 112fac5 into nanoframework:main Apr 7, 2026
27 checks passed
@josesimoes
josesimoes deleted the fix-socket branch April 7, 2026 13:39
@josesimoes josesimoes mentioned this pull request Apr 7, 2026
4 of 13 tasks
josesimoes added a commit to josesimoes/nf-interpreter that referenced this pull request Sep 24, 2026
- Restore sock_set_errno in the ESP32 socket API.
- Clear the stale EINPROGRESS after connect, so real errors are no longer reported as EWOULDBLOCK.
- These were lost in lwIP re-import at nanoframework#3289.
- Also add a note to re-apply this after future upstream updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Common libs Everything related with common libraries Type: bug Type: dependencies Pull requests that update a dependency file(s) or version Type: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants