dotnet / dotnet/extensions

[Telemetry Logging] LegacyTagJoiner and ModernTagJoiner could cause OutOfMemoryException

Open
#7,505 0 comments 0 reactions 1 assignee Claimed by @iliar-turdushev View on GitHub
area-telemetry bug
Dominant language
C#
Stars
3.2k
Forks
894
Avg merge
1d 12h
Merged PRs (30d)
23

Description

### Description

`LegacyTagJoiner` dynamically calculates its `Count` property and its `GetEnumerator()` method iterates over items that could change dynamically during the iteration. This could lead to OOM if not used properly. See reproduction steps.

The same issue exists in `ModernTagJoiner` as well.

### Reproduction Steps

Add and run the following unit test. The memory will start growing and eventually an OOM will be thrown.

```csharp
[Fact]
public static void OutOfMemoryTest()
{
var joiner = new ExtendedLogger.LegacyTagJoiner { StaticTags = [] };
joiner.SetIncomingTags(
new List>
{
new("K1", "V1"),
new("K2", "V2"),
}.AsReadOnly());
var source = joiner.Append(new KeyValuePair("K3", "V3"));
joiner.EnrichmentTagCollector.AddRange(source);
}
```

When `AddRange` enumerates `source` that references the `joiner` while adding to `_extraTags` (an inner collection in `joiner` holding its items), `Count` increases with each added item --> the enumerator loop never terminates --> unbounded memory growth until OOM.

### Expected behavior

OOM shouldn't be thrown, items from the `source` should be added to the `joiner`.

### Actual behavior

An infinite loop adding items from the `source` to the `joiner` causes OOM.

### Regression?

_No response_

### Known Workarounds

If you materialize the `source` by invoking `ToArray` or `ToList` the items will be added to the `joiner` and OOM will not be thrown.

### Configuration

_No response_

### Other information

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.