ArrayMemoryPool.ArrayMemoryPoolBuffer implementation of ref counting isn't thread safe

[davidfowl]

We're not using interlocked for some reason here and we should be https://github.com/dotnet/corefx/blob/865f08623bd493fb6d239530e00db29bf6736330/src/System.Memory/src/System/Buffers/ArrayMemoryPool.ArrayMemoryPoolBuffer.cs#L80-L98. This could potentially lead to leaks.

/cc @jkotas @atsushikan @ahsonkhan @pakrym @KrzysztofCwalina

[pakrym]

This needs to get into preview2

[stephentoub]

See the discussion here:
dotnet/corefx#26563 (review)

[davidfowl]

These counts need to be accurate, it's not only about use after free (I missed that discussion), the way pipelines uses them relies on this reference counting to return to the pool. If it stays this way we'll keep our own reference count and ignore the Retain/Release semantics of OwnedMemory<T> which would make me a sad 🐼

[jkotas]

This could potentially lead to leaks.

From the usage pattern you are describing, it can also lead to use-after-free bugs. Use-after-free bugs are security bugs - much worse than leaks.

[davidfowl]

Agreed 😄. Here's a related bug https://github.com/dotnet/corefx/issues/27544.

[KrzysztofCwalina]

@atsushikan, is this fixed? If yes, please close.

[ghost]

@davidfowl - Pls confirm that dotnet/corefx#27615 addresses your needs (and that we've decided what the semantics are :))

[davidfowl]

Yep, this is good to close