Calling EventSource.Write with data containing an array of string throws on .net Core 2.1, works on .net Core 2.0

[smmalis37]

I have code that calls EventSource.Write and passes in a field that is an array of strings. The code looks similar to:

public static void LogFoo(string[] foo) {
    EventSource logger = new EventSource(...);
    logger.Write("...", options, new { Foo = foo });
}

This code works without issue when run targeting any version of .net Framework I've tested, and when targeting netcoreapp2.0. It does not work with netcoreapp2.1 with runtime version v2.1.300, instead throwing a System.NotSupportedException: 'Arrays of null-terminated string are not supported.' . While the message is pretty clear, I'm wondering if this functionality regression was intentional?

[davidmatson]

Yeah, having a minor version include breaking changes isn't good semver, so I'm hoping that's not intentional.

@brianrob, would you happen to know what the root cause might be? (Since you were the last one to edit that line, though admittedly just changing how the error lookup happens.)

[brianrob]

This is likely caused by https://github.com/dotnet/coreclr/pull/16672 where we switch from writing to ETW using counted strings to null-terminated strings when using the Write APIs. The message was always there, but because we were using counted strings we never hit it. Let me see what can be done to fix this.

[vancem]

including @jorive @noahfalk

[tommcdon]

@noahfalk any objections moving this to future?

[noahfalk]

I'd still like to get this addressed in 3.0 if we can

[josalem]

After some research, changing this in a fully supportable manner (not just a workaround) would involve non-trivial changes to the serialization format used. This could introduce instability in both the source and reader that is unwanted going into the 3.0 release. I'm going to move this to future for now. @brianrob, you're most likely much more familiar with these code paths, if you think this would be a relatively straightforward update to the format, please let me know.

[noahfalk]

@josalem - can you elaborate on your investigation results such as what options you looked at and what changes/risks those options cause? Since we've only marked this Future rather than closing it suggests that someone is going to be re-opening this investigation and it would be a shame if they had to start over from the beginning.

[brianrob]

@josalem, I suspect you are right that this is not going to be trivial. It stems from the fact that I was trying to unify the way we serialize strings between TraceLogging ETW and EventPipe so that I could add support for TraceLogging in EventPipe without having to support two deserialization schemes within TraceEvent. TraceLogging ETW previously used counted strings and had support for arrays of counted strings but when I switched things over to NULL-terminated strings, I did not realize, nor have a test case that covered arrays of strings. The code had always been written to disallow this, but the path was never hit.

I suspect that it is possible to implement this without too much trouble, but I don't know what the encoding would look like. I don't believe that this would require fundamental changes to EventSource, but would require that we understand what the encoding should look like for supporting this on the ETW side, and then making sure that this works on the EventPipe side as well.

If you want to dig into this more, let me know and I can connect you with the right person who can tell us what the encoding should look like.

[josalem]

@noahfalk, as @brianrob points out, it looks to me like the majority of this work is going to be nailing down a stable encoding for arrays of "primitive" types (primitive in regards to what is already serializable by ETW/EventPipe). That, in and of itself, shouldn't be too difficult (famous last words and all that), but will require updates to the serializers for ETW and EventPipe, as well as updating the deserializers.

I feel there is higher priority work that will introduce less instability that can be completed with the timeframe we're looking at. I do think this work is worth implementing, as having arrays of "things" inside events would be very useful. I'm open to moving it back into 3.0, but I think IPC work should take priority since the current encoding seems to have only recently stabilized. @brianrob, since there is going to be work surrounding serialization in general I think it would be worth syncing offline to go over the moving parts.

[brianrob]

Sure, @josalem. Happy to chat.

[tommcdon]

@davmason is this fixed in 5.0?

[davmason]

This is not fixed in 5.0. The work I did recently for array types would allow us to put an array of strings in the file, but there is some code in EventSource that explicitly checks for arrays of null terminated strings and throws an exception. It may be as simple as removing that code, I'm looking further

[davmason]

After some further research, this works because you can pass arbitrary sized collections of strings to EventSource, but the specific example above doesn't work.

Anonymous types with arrays as arguments are broken for all types. I.e. if you pass new { args = new int[] { 1, 2, 3, 4 }} as a parameter to WriteEvent it will also fail.

I suspect the code above is doing that because WriteEvent(1, new string[] { "sample", "array" }); will fail because the C# compiler resolves to the WriteEvent(int eventId, params object[]? args) overload and the strings are passed as individual arguments instead of one array argument.

If you use the IEnumerable overload instead it will correctly send the event in .net 5:

        [Event(2, Level = EventLevel.LogAlways)]
        public void StringIEnumerableEvent(IEnumerable<string> args)
        {
            WriteEvent(2, args);
        }

This is not an issue for built in types (int, double, etc) arrays. I am not a C# language spec expert, but I suspect the issue is that string is an object and ints, etc, are value types so they won't use the object[] params overload.

The real issue here is the fact that anonymous types don't work with array types. I think this will require enough changes to EventSource that it should probably be pushed out to 6.0.

[josalem]

@davmason was this fixed in 5.0?

[davmason]

No, I explained the issue above but I haven't worked on it. I don't think there really is an issue with the way arrays of strings work, it's just that the EventSource APIs have some corner cases that are unexpected.

[mslukebo]

Have there been any updates on this issue? I am trying to log an array of strings from a .NET 6 application, but I'm hitting this exception. My IEnumerable<string> is inside EventData struct, so I cannot manually call the appropriate Write method since I'm writing the entire struct.