Conversation
Clarified the discussion on resource lifetime and reflection in the context of memory safety.
| These data races are considered fundamental to the .NET type system. The safe subset of C# does not protect against them. | ||
|
|
||
| - Resource lifetime. Some code patterns, like object pools, require manual lifetime management. When this management is done incorrectly bad behaviors can occur, including improper memory reuse. Notably, none of those behaviors include invalid memory access, although it can include symptoms that look like memory corruption. Because invalid memory access is not possible, this is considered safe. | ||
| - Resource lifetime. Some code patterns, like object pools, require manual lifetime management. When this management is done incorrectly bad behaviors can occur, including improper memory reuse. Notably, none of those behaviors include invalid memory access, although it can include symptoms that look like memory corruption. Because invalid memory access is not possible, this is considered safe. Any resource lifetime issues that can cause invalid memory access _are_ considered unsafe. |
There was a problem hiding this comment.
I disagree with this position.
If we are not going to address memory safety issues of the global ArrayPool in this project, we have failed.
|
|
||
| - Resource lifetime. Some code patterns, like object pools, require manual lifetime management. When this management is done incorrectly bad behaviors can occur, including improper memory reuse. Notably, none of those behaviors include invalid memory access, although it can include symptoms that look like memory corruption. Because invalid memory access is not possible, this is considered safe. | ||
| - Resource lifetime. Some code patterns, like object pools, require manual lifetime management. When this management is done incorrectly bad behaviors can occur, including improper memory reuse. Notably, none of those behaviors include invalid memory access, although it can include symptoms that look like memory corruption. Because invalid memory access is not possible, this is considered safe. Any resource lifetime issues that can cause invalid memory access _are_ considered unsafe. | ||
|
|
There was a problem hiding this comment.
| Mutable global state in .NET makes it possible to create APIs that are prone to produce symptoms identical to memory corruptions caused by lifetime management bugs in user code. `ArrayPool<byte>.Shared` is the prime example of such API. Even though API implementation may be 100% safe code, we will take a discretion to mark such APIs in core libraries as unsafe and propose safe alternatives to replace the most common usage patterns. |
This is my counterproposal.
There was a problem hiding this comment.
Why not adjust our safety rules to include memory reuse from ArrayPool? That is, rather than create an exception for ArrayPool without putting it in the rules, I'd be OK just putting the ArrayPool approach in our rules directly.
There was a problem hiding this comment.
Just curious what would be a safe alternative? Isn't the issue fundamental to array pooling, the caller may return and continue use?
There was a problem hiding this comment.
Just curious what would be a safe alternative?
There are common patterns that we can introduce safe alternatives for. A local scratch buffer pattern is the prime example (dotnet/runtime#52065).
There was a problem hiding this comment.
Why not adjust our safety rules to include memory reuse from ArrayPool?
We have more global pool-like APIs. For example, System.Runtime.InteropServices.GCHandle. It happens to be implemented in C++, but it would be possible to re-implement it in safe C# and ensure that accessing the handle does not ever perform invalid memory access even when user code has use-after-free or double-free bugs. A safe implementation like that would not move the needle on GCHandle safety. It would be as unsafe as it is today.
There was a problem hiding this comment.
The problematic API pattern is: Global, typically low-level, API with manual lifetime management, that many higher-level components depend on.
| - Resource lifetime. Some code patterns, like object pools, require manual lifetime management. When this management is done incorrectly bad behaviors can occur, including improper memory reuse. Notably, none of those behaviors include invalid memory access, although it can include symptoms that look like memory corruption. Because invalid memory access is not possible, this is considered safe. | ||
| - Resource lifetime. Some code patterns, like object pools, require manual lifetime management. When this management is done incorrectly bad behaviors can occur, including improper memory reuse. Notably, none of those behaviors include invalid memory access, although it can include symptoms that look like memory corruption. Because invalid memory access is not possible, this is considered safe. Any resource lifetime issues that can cause invalid memory access _are_ considered unsafe. | ||
|
|
||
| - Reflection. Reflection is a known hole in the current unsafe model. Reflection can be used to call unsafe methods or access unsafe properties without the reflection code containing any unsafe blocks. This may be addressed a future proposal. |
There was a problem hiding this comment.
We should also address handles (OS handles in particular) since they are in similar category: They are not pure memory but produce same types of corruption. I assume that we are going to mark APIs that operate on raw (OS) handles without enforced lifetime management as unsafe. For example, public SafeFileHandle(IntPtr preexistingHandle, bool ownsHandle).
There was a problem hiding this comment.
Yeah, I expect that there's some potential for memory corruption in these cases, somewhere, so they would end up marked in the same way.
There was a problem hiding this comment.
The first order failure for OS handles lifetime issues is not a memory corruption. It is a syscall returning invalid handle error or unexpected data (e.g. you end up reading from a different file than the one you have opened originally). The invalid memory access can be a second order effect caused by the unexpected data.
It is very similar to the array pool. The first order failure for array pool lifetime issues is not a memory corruption. It is an array containing unexpected data. The invalid memory access can be a second order effect caused by the unexpected data.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
3 participants
Clarified the discussion on resource lifetime and reflection in the context of memory safety.