Skip to content

Comments

Add explicit Size parameter to AsyncCancel() and AsyncWait() APIs#1160

Merged
paul-manias merged 1 commit intomasterfrom
test/async-api-size-params
Feb 21, 2026
Merged

Add explicit Size parameter to AsyncCancel() and AsyncWait() APIs#1160
paul-manias merged 1 commit intomasterfrom
test/async-api-size-params

Conversation

@paul-manias
Copy link
Member

Summary

  • Replace zero-terminated OBJECTID arrays with explicitly sized arrays in AsyncCancel() and AsyncWait() for type safety and correct Tiri array interoperability
  • Add AsyncWait() XML documentation
  • Update function definitions, prototypes, headers, IDL, and Tiri bindings across 7 files

Test plan

  • Build and verify compilation succeeds
  • Run existing async-related Flute tests
  • Verify Tiri scripts calling AsyncCancel() and AsyncWait() work with the new sized-array signatures

🤖 Generated with Claude Code

Replace zero-terminated OBJECTID arrays with sized arrays for type safety and Tiri interoperability. Adds AsyncWait() XML documentation and updates all bindings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@paul-manias
Copy link
Member Author

@claude

@claude
Copy link

claude bot commented Feb 21, 2026

Claude finished @paul-manias's task —— View job


Code Review Analysis ✅

Todo List

  • Read and analyze all changed files
  • Review API signature changes for type safety
  • Check XML documentation updates
  • Verify Tiri bindings consistency
  • Analyze implementation changes
  • Provide comprehensive review feedback

APPROVE - Strong Type Safety Improvement

This PR successfully addresses a critical API design flaw by replacing zero-terminated OBJECTID arrays with explicitly sized arrays. The changes are comprehensive, well-executed, and maintain backward compatibility through proper Tiri bindings.

🎯 Key Strengths

  1. Type Safety Enhancement: The change from FD_PTR to FD_ARRAY|FD_OBJECTID with FD_ARRAYSIZE provides much stronger type checking and eliminates potential buffer overflow risks from malformed arrays.

  2. Comprehensive Implementation: All necessary layers have been updated consistently:

    • C function signatures: src/core/lib_objects.cpp:932,1027
    • Header files: include/kotuku/modules/core.h:2019,2021
    • Prototypes: src/core/prototypes.h:93,95
    • IDL definitions: src/core/data_functions.c:18,20
    • Tiri bindings: src/tiri/tiri_async.cpp:383
    • XML documentation: docs/xml/modules/core.xml:334-337,367-369
  3. Improved Tiri Interoperability: The Tiri implementation now correctly handles both single objects and object arrays without zero-termination requirement, making the API more intuitive for scripting.

🔍 Technical Analysis

AsyncCancel() Changes (lib_objects.cpp:932-955):

  • Proper bounds checking with i < Size instead of relying on zero termination
  • Maintains thread safety through existing mutex locks
  • Clean removal of redundant bracing in conditional

AsyncWait() Changes (lib_objects.cpp:1027-1055):

  • Consistent parameter handling with size-based iteration
  • Proper documentation addition for the new Size parameter
  • Thread-safe implementation preserved

Tiri Binding Updates (tiri_async.cpp:383):

  • Correctly passes ids.size() instead of assuming zero termination
  • Maintains existing error handling patterns
  • Removes unnecessary zero-termination step

📚 Documentation Quality

The XML documentation additions are excellent:

  • Clear parameter descriptions with proper <code> formatting
  • Maintains consistent style with existing documentation
  • Comprehensive error code documentation for AsyncWait()

⚠️ Minor Observations

  1. Moved Function Location: The async_wait_callback() function was moved from line 1007 to line 111. While this doesn't affect functionality, it's worth noting for reviewers.

  2. No Backward Compatibility Issues: The size parameter addition doesn't break existing C++ code since callers must be updated to provide the size, making the breaking change explicit and safe.

🧪 Test Recommendations

The test plan covers the essential areas. I recommend also testing:

  • Empty arrays (Size=0) to ensure graceful handling
  • Large arrays to verify performance remains acceptable
  • Error conditions in Tiri scripts to ensure proper exception propagation

Verdict

This is a well-executed API improvement that enhances type safety without compromising functionality. The implementation is thorough, consistent, and properly documented. The breaking change is justified by the significant safety improvements it provides.

Status: APPROVED ✅

@paul-manias paul-manias merged commit 7415ff1 into master Feb 21, 2026
3 checks passed
@paul-manias paul-manias deleted the test/async-api-size-params branch February 21, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant