Skip to content

Fix generated string-sequence marshalling for messages with multiple string[]/wstring[] fields#56

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-publish-null-reference-exception
Draft

Fix generated string-sequence marshalling for messages with multiple string[]/wstring[] fields#56
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-publish-null-reference-exception

Conversation

Copilot AI commented Apr 17, 2026

Copy link
Copy Markdown

Publishing could fail for messages containing multiple string-array fields (including nested messages), with intermittent NullReferenceException surfacing from publisher buffer creation. The failure pattern matched native message state corruption during generated write-marshalling of dynamic string sequences.

  • Root cause in generated WriteTo(ref Priv ...) marshalling

    • For variable-length string[] / wstring[] fields, generated code reassigned native sequence structs directly.
    • Existing native sequence storage was not explicitly finalized before reallocation, which can leave invalid native state when multiple string-sequence fields are present.
  • Generator change

    • Updated MessageClassBuilder emission logic for dynamic string arrays to finalize prior native sequence state before allocating replacement storage:
      • CStringSequence: emit priv.<field>.Dispose(); before new CStringSequence(...)
      • U16StringSequence: emit priv.<field>.Dispose(); before new U16StringSequence(...)
    • Change is scoped only to variable-length string/wstring array branches; other field marshalling paths are unchanged.
  • Effect

    • Generated message writers now perform safe replace semantics for native string sequences, avoiding invalid state when multiple string-array members are populated in the same message.
// now emitted for dynamic string[] fields
priv.Works.Dispose();
priv.Works = new global::Rosidl.Runtime.Interop.CStringSequence(this.Works.Length);
var Works_span = priv.Works.AsSpan();
for (int __i = 0; __i < this.Works.Length; __i++)
{
    Works_span[__i].CopyFrom(this.Works[__i], textEncoding);
}

Copilot AI linked an issue Apr 17, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix publish failure with multiple string arrays Fix generated string-sequence marshalling for messages with multiple string[]/wstring[] fields Apr 17, 2026
Copilot AI requested a review from noelex April 17, 2026 16:17
@noelex noelex requested a review from Copilot April 17, 2026 16:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the C# message code generator to prevent native string-sequence state corruption when marshalling messages that contain multiple variable-length string[] / wstring[] fields.

Changes:

  • For dynamic (variable-length) string[] fields, generated WriteTo(ref Priv ...) now finalizes the existing CStringSequence via Dispose() before allocating a replacement sequence.
  • For dynamic (variable-length) wstring[] fields, generated WriteTo(ref Priv ...) now finalizes the existing U16StringSequence via Dispose() before allocating a replacement sequence.

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

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.

Multiple string arrays

3 participants