Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent/contradictory statements on StringBuilder marshaling in PInvoke #47735

Closed
macrogreg opened this issue Feb 2, 2021 · 4 comments · Fixed by dotnet/docs#25003
Closed
Assignees
Labels
area-System.Runtime.InteropServices documentation Documentation bug or enhancement, does not impact product or test code
Milestone

Comments

@macrogreg
Copy link

macrogreg commented Feb 2, 2021

There are a few places in the docs that seem to state inconsistent/contradictory facts on how StringBuilder is treated during PInvoke marshaling. Please clarify which is correct, and adjust the docs as appropriate to reduce future confusion.

Some docs state that when a StringBuilder is passed by value to PInvoke, a pointer to the internal buffer is passed to native, and no copy occurs. E.g.:

When a System.Text.StringBuilder is passed by value, the marshaler passes a reference to the internal buffer of the StringBuilder directly to the caller. The caller and callee must agree on the size of the buffer. The caller is responsible for creating a StringBuilder of adequate length. The callee must take the necessary precautions to ensure that the buffer is not overrun. StringBuilder is an exception to the rule that reference types passed by value are passed as In parameters by default. It is always passed as In/Out.

  • Docs > .NET > .NET Framework > Interoperate with unmanaged code > Default Marshaling for Strings
    sub-section "Fixed-Length String Buffers":

[...] The solution is to pass a StringBuilder buffer as the argument instead of a String. A StringBuilder can be dereferenced and modified by the callee, provided it does not exceed the capacity of the StringBuilder. It can also be initialized to a fixed length. [...]

Conversely, other docs explicitly recommend avoiding the use of StringBuilder stating that a copy of the internal buffer always occurs. E.g.:

  • Docs > .NET > .NET fundamentals > Native interoperability best practices
    sub-section "String parameters":

AVOID StringBuilder parameters. StringBuilder marshaling always creates a native buffer copy. [...]

  • Docs > .NET > .NET fundamentals > CA1838: Avoid StringBuilder parameters for P/Invokes
    entire page:

Marshaling of StringBuilder always creates a native buffer copy, resulting in multiple allocations for one P/Invoke call. To marshal a StringBuilder as a P/Invoke parameter, the runtime will:
- Allocate a native buffer.
- If it is an In parameter, copy the contents of the StringBuilder to the native buffer.
- If it is an Out parameter, copy the native buffer into a newly allocated managed array.
[...]

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 2, 2021
@jkotas jkotas added area-System.Runtime.InteropServices documentation Documentation bug or enhancement, does not impact product or test code labels Feb 2, 2021
@macrogreg
Copy link
Author

Btw, it is clear that in order for a no-copy marchaling to be feasible, the StringBuilder must be marshaled as LPWStr, e.g.

[DllImport("Foo.dll")]
public static extern UInt32 ReadMyStringValue(
                [MarshalAs(UnmanagedType.LPWStr)] StringBuilder strBuffer,
                UInt32 strBufferLen,
                ref UInt32 actualStrValueLen);

So, if the best practice is different depending on the Encoding used, please clarify it accordingly in the docs (and here).

Thank you!

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Feb 9, 2021

Thanks for pointing this out @macrogreg. Your note about encoding is accurate. The copying details are typically around encoding although there are issues relating to terminating nulls. We will update the docs.

/cc @jkoritzinsky @elinor-fung

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Feb 9, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 6.0.0 milestone Feb 9, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added this to To do in Interop-CoreCLR 6.0 via automation Feb 9, 2021
@macrogreg
Copy link
Author

Thank you, @AaronRobinsonMSFT .
I understand that updating the docs can be a process that can take some time. In the meantime, could you please clarify what the actual situation is? :

  • If a StringBuilder is passed to a PInvoke as an LPWStr in a manner described in the above comment, will or will not a buffer mem copy take place?

  • What are the terminating null issues you are mentioning? I realize that the buffer length count can be confusing in respect to whether or not the terminating null is counted, but I do not understand in what exact way it related to the mem-copy.

Thanks!

@jkoritzinsky jkoritzinsky self-assigned this Jul 6, 2021
@jkoritzinsky jkoritzinsky moved this from To do to In progress in Interop-CoreCLR 6.0 Jul 6, 2021
Interop-CoreCLR 6.0 automation moved this from In progress to Done Jul 7, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.InteropServices documentation Documentation bug or enhancement, does not impact product or test code
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants