Skip to content

Conversation

@josesimoes
Copy link
Member

@josesimoes josesimoes commented Dec 11, 2025

Description

  • Interation of assemblies ans spawning of cctor is now inside a loop to take care of nested dependencies and the need for multiple passes until all are satisfed.

Motivation and Context

  • When testing more complex applications with more intricate dependencies, it was determined that some .cctor weren't running, thus causing issues.
  • Rework on this code was made in Implement spawning static constructor for closed generic types #3235, but the existing tests then all passed. That was because the level of tangled dependencies could be satisfied. As the number of assemblies grows and the dependencies as more nested, the shortcomings showed up.

How Has This Been Tested?

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).
  • [xI 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

  • Refactor

    • Enhanced static constructor spawning with multi-pass mechanism and progress tracking.
    • Expanded dependency validation for regular and generic type static constructors.
  • Style

    • Updated comment formatting.

✏️ Tip: You can customize this high-level summary in your review settings.

- Interation of assemblies ans spawning of cctor is now inside a loop to take care of nested dependencies and the need for multiple passes until all are satisfed.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025

Walkthrough

A multi-pass loop mechanism was introduced to the static constructor spawning system in the CLR execution module. The mechanism iterates across assemblies, scheduling regular and generic type static constructors in successive passes until no further progress is made, with dependency satisfaction checks expanded to account for cross-assembly constructor execution ordering.

Changes

Cohort / File(s) Summary
Static constructor spawning mechanism
src/CLR/Core/Execution.cpp
Refactored SpawnStaticConstructor to use a multi-pass loop with progress tracking via anySpawned boolean. Expanded dependency checks for regular and generic static constructors to only execute when dependent assemblies have completed their regular static constructor passes. Loop continues until no new constructors are spawned in a full pass. Included minor comment text updates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Multi-pass loop logic: Verify correctness of loop termination condition and the relationship between anySpawned flag and loop continuation.
  • Dependency satisfaction checks: Confirm that dependency checks for both regular and generic static constructors correctly determine when scheduling should occur and that the pass-based approach handles cross-assembly dependencies.
  • Control flow interactions: Ensure the scheduling return conditions (true vs. false) properly interact with progress tracking and loop continuation.

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 'Fix spawning static constructors' directly and clearly summarizes the main change in the pull request, which introduces a multi-pass mechanism to fix issues with static constructor spawning in complex assembly dependency scenarios.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68fff31 and 70f8918.

📒 Files selected for processing (1)
  • src/CLR/Core/Execution.cpp (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_P4_UART)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_ETHERNET_KIT_1.2)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_S3_ALL)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_H2_THREAD)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C6_THREAD)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C3)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_BLE_REV0)
  • GitHub Check: nf-interpreter (Build_STM32_targets ST_STM32F769I_DISCOVERY)
  • GitHub Check: nf-interpreter (Build_WIN32_nanoCLR)
  • GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_PSRAM_REV0)
  • GitHub Check: nf-interpreter (Build_NXP_targets NXP_MIMXRT1060_EVK)
  • GitHub Check: nf-interpreter (Build_TI_SimpleLink_targets TI_CC1352R1_LAUNCHXL_915)
  • GitHub Check: nf-interpreter (Build_Azure_RTOS_targets SL_STK3701A)
  • GitHub Check: nf-interpreter (Build_STM32_targets ST_STM32F429I_DISCOVERY)
  • GitHub Check: nf-interpreter (Check_Code_Style)
  • GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets MXCHIP_AZ3166)
  • GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALX)
  • GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALTHREE)
  • GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
  • GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (1)
src/CLR/Core/Execution.cpp (1)

1167-1245: Excellent multi-pass solution for handling nested assembly dependencies.

The implementation correctly addresses the issue where static constructors weren't running in applications with complex dependency chains. Key strengths:

  1. Progress tracking: The anySpawned flag ensures the loop terminates once no further progress can be made in a full pass through all assemblies.

  2. Correct dependency ordering: Regular constructors are spawned only when all dependent assemblies have executed their regular constructors, and generic constructors wait for regular constructors of dependencies to complete.

  3. Early return optimization: When a constructor is scheduled (helper returns true), the function returns immediately to allow the cctor thread to run, resuming iteration after completion.

  4. Proper control flow: The continue at line 1208 correctly skips the generic constructor check when regular constructors haven't been executed yet, since generic constructors also depend on the same dependencies being satisfied.

The worst-case complexity is O(N²) for N assemblies in a dependency chain, but this is acceptable for initialization code and bounded by the number of assemblies.


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 Dec 11, 2025
@josesimoes
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@josesimoes josesimoes merged commit b295724 into nanoframework:develop Dec 11, 2025
26 checks passed
@josesimoes josesimoes deleted the fix-spawning-cctors branch December 11, 2025 23:07
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants