Skip to content

Commit 8c29f48

Browse files
Copilotjkotas
andauthored
Merge remote-tracking branch 'origin/main' into remove-postcondition-return
# Conflicts: # src/coreclr/vm/assemblyspec.hpp Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
2 parents 3d1150c + d38d8f7 commit 8c29f48

536 files changed

Lines changed: 20901 additions & 10408 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/agents/extensions-reviewer.md

Lines changed: 245 additions & 0 deletions
Large diffs are not rendered by default.

.github/copilot-instructions.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Based on file paths you will modify:
8484

8585
### Step 2: Run the Baseline Build (from repo root)
8686

87-
**First, checkout the `main` branch** to establish a known-good baseline, then run the appropriate build command:
87+
From the repo root, on the branch you intend to modify, ensure you have a clean working tree (no uncommitted changes) at the current HEAD, then run the appropriate build command **before making any code changes**:
8888

8989
| Component | Command |
9090
|-----------|---------|
@@ -108,7 +108,7 @@ export PATH="$(pwd)/.dotnet:$PATH"
108108
dotnet --version # Should match sdk.version in global.json
109109
```
110110

111-
**Only proceed with changes after the baseline build succeeds.** If it fails, report the failure and stop. After the baseline build, switch back to your working branch before making changes.
111+
**Only proceed with changes after the baseline build succeeds.** If it fails, report the failure and stop.
112112

113113
---
114114

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
applyTo: "src/libraries/System.IO.Compression*/**"
3+
---
4+
5+
# System.IO.Compression — Folder-Specific Guidance
6+
7+
## Format Specification Correctness (D12)
8+
9+
- ZIP64 extensions must be used for files over 4GB — extra field sizes, offsets, and header values must use 64-bit fields when the 32-bit range is exceeded
10+
- Compression levels must align with native library semantics — verify enum-to-native mapping is correct
11+
- New compression format support (e.g., zstd in ZIP) must include a feature switch for trimming/AOT and an explicit opt-in mechanism
12+
- Decompression must handle concatenated payloads and partial reads — the decompressor must not assume a single contiguous compressed stream
13+
- Breaking changes to format handling must be documented and include migration guidance
14+
15+
## Security
16+
17+
- Maximum decompressed size limits must be configurable to prevent zip-bomb attacks, following the existing deflate size limit pattern
18+
- Archive extraction must validate entry paths to prevent path traversal attacks (entries with `../` segments)
19+
20+
## Performance & Allocation (D5)
21+
22+
- Use `ArrayPool<byte>` for variable-size compression/decompression buffers — return buffers in finally blocks
23+
- Avoid allocating excessively large fixed buffers per operation (100KB+ per compression operation is expensive)
24+
- Pin buffers for the duration of native I/O operations
25+
- Hot paths must avoid per-operation allocations — prefer pooled buffers and cached delegates
26+
- Closures that capture state on hot paths must be eliminated — use static lambdas with explicit state
27+
28+
## Async Operations
29+
30+
- Async compression/decompression must not perform the actual compression work synchronously before the first await
31+
- Sync and async code paths must share non-trivial logic through common helpers to prevent divergence
32+
33+
## Cross-Platform Metadata (D19)
34+
35+
- Archive extraction must preserve or correctly translate platform-specific metadata — Unix execute permissions, symlinks, and hidden file attributes
36+
- File path operations within archives must use forward slashes as the archive-internal separator per the ZIP specification
37+
- Tests must verify metadata round-trip on both Windows and Unix platforms
38+
39+
## Native Interop
40+
41+
- Native library updates (brotli, zlib, zstd) must be tracked and the managed wrapper updated accordingly
42+
- Use `LibraryImport` (source-generated) for new P/Invoke declarations
43+
- SafeHandle-derived types must be used for native compression handles — never store raw IntPtr
44+
- Native error codes must be mapped to appropriate .NET exceptions with the native error code preserved
45+
46+
## Error Handling (D9)
47+
48+
- Exceptions must be the most specific applicable type — `InvalidDataException` for corrupt archives, `IOException` for I/O failures, with actionable context (entry name, expected vs actual values)
49+
- Operations on streams that may not support Length/Seek must be guarded appropriately
50+
51+
## Interoperability Testing (D10)
52+
53+
- Tests must use archive files created by external tools — not just round-trip tests with the same .NET implementation
54+
- Test with archives from multiple platforms and compression libraries to verify cross-tool compatibility
55+
- Cover edge cases: empty archives, many small entries, entries at size boundaries (4GB, uint.MaxValue)
56+
- Dispose behavior must be tested — verify resources are released and post-disposal operations throw
57+
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
applyTo: "src/libraries/Microsoft.Extensions.Caching*/**"
3+
---
4+
5+
# Microsoft.Extensions.Caching — Folder-Specific Guidance
6+
7+
## Cache Key Correctness (D20)
8+
9+
- Cache keys must incorporate all inputs that affect the cached result — including format versions, serialization options, and any parameters that change the output
10+
- Key generation must be deterministic and stable across process restarts for distributed caches
11+
- Verify that key composition does not create collisions for distinct inputs
12+
- Cache miss must be distinguishable from a cached null value
13+
14+
## Eviction Policies (D20)
15+
16+
- Cache eviction must consider TTL, priority, estimated object size, and memory pressure — not just LRU
17+
- Expose eviction callbacks so consumers can observe and react to entry removal
18+
- Memory pressure-based eviction should clear stale references to avoid retaining unused objects
19+
20+
## Stampede Prevention (D20)
21+
22+
- When a cache entry expires, only one caller should recompute the value while others wait (thundering herd mitigation)
23+
- HybridCache (available in .NET 9+) and similar patterns should use a single-flight mechanism for concurrent requests to the same key
24+
25+
## Performance & Allocation (D5)
26+
27+
- Closures for cache factory methods must not allocate on every cache access — cache the delegate or use a static lambda with explicit state
28+
- Distributed cache serialization must handle large objects efficiently — consider compression and streaming for values over 100KB
29+
- In-memory cache operations must avoid holding locks during expensive value computation
30+
- Hot paths must avoid per-operation allocations — prefer pooled buffers and cached delegates
31+
32+
## Thread Safety (D6)
33+
34+
- `MemoryCache` and `HybridCache` (available in .NET 9+) are expected to be used concurrently — all access patterns must be thread-safe
35+
- Entry creation and eviction callbacks may execute on different threads — do not assume single-threaded access
36+
37+
## Distributed vs In-Memory
38+
39+
- `IDistributedCache` implementations must handle serialization/deserialization correctly and document size limits
40+
- Test with both in-memory and distributed backing stores — behavior differences must be accounted for
41+
- Distributed cache key expiration semantics may differ from in-memory — document and test accordingly
42+
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
applyTo: "src/libraries/Microsoft.Extensions.Http*/**,src/libraries/Microsoft.Extensions.FileProviders*/**,src/libraries/Microsoft.Extensions.Primitives*/**"
3+
---
4+
5+
# Microsoft.Extensions Common Libraries — Folder-Specific Guidance
6+
7+
Covers `Microsoft.Extensions.Http`, `Microsoft.Extensions.FileProviders`, and `Microsoft.Extensions.Primitives`.
8+
9+
## Extensions.Http (D17, D7)
10+
11+
- `HttpClientFactory` registration helpers must use `TryAdd` patterns to avoid overriding user-configured handlers
12+
- `DelegatingHandler` chains must correctly propagate disposal — document ownership of inner handlers
13+
- Connection pooling and lifetime settings (`PooledConnectionLifetime`) must be documented when composing `HttpClient` configurations
14+
- Ensure thread-safe access for `HttpClient` instances shared via factory — use `CompareExchange` for lazy initialization
15+
- Avoid per-operation allocations in handler pipelines — cache delegates and reuse handler instances where possible
16+
- All HttpClientFactory types must be trim-safe and NativeAOT-compatible
17+
18+
## Extensions.FileProviders (D10, D4)
19+
20+
- `PhysicalFileProvider` must handle concurrent file operations — use `FileShare.Delete` when reading files that may be rotated
21+
- File provider ownership is not transferred to `IConfigurationRoot` or `IConfigurationProvider` — document the ownership boundary
22+
- `IChangeToken` implementations must correctly handle callback registration and disposal
23+
- Polling-based file watchers must have configurable intervals and must clean up on disposal
24+
- Avoid unnecessary `ToArray()`/`ToList()` calls — prefer lazy enumeration with yield return when callers do not need materialized collections
25+
- Directory enumeration order changes are breaking changes — test ordering expectations
26+
27+
## Extensions.Primitives (D1, D5)
28+
29+
- `StringSegment` must provide string-equivalent operations — missing methods should maintain API parity with `string`
30+
- `ChangeToken.OnChange` registrations must be disposed when the owning component is disposed to prevent leaks
31+
- Implicit operators (e.g., string to `StringSegment`) must be documented and must not allocate on the conversion path
32+
- `GetHashCode` implementations must not allocate — use span overloads rather than allocating substrings
33+
- Avoid `Unsafe.As` casts when a standard cast is equally cheap (e.g., object to string)
34+
- Performance-sensitive code paths must avoid closures, string allocations, and boxing
35+
36+
## Cross-Cutting
37+
38+
- All types in these libraries must be trim-safe and NativeAOT-compatible
39+
- Public API changes must be evaluated for backward compatibility — these are foundational types with many downstream consumers
40+
- Abstractions (interfaces, base classes) belong in `*.Abstractions` packages; implementations in concrete packages
41+
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
applyTo: "src/libraries/Microsoft.Extensions.Configuration*/**"
3+
---
4+
5+
# Microsoft.Extensions.Configuration — Folder-Specific Guidance
6+
7+
## Configuration Binding (D8)
8+
9+
- Configuration key lookups are case-insensitive — all switch statements and dictionary lookups over config keys must use `StringComparison.OrdinalIgnoreCase`
10+
- The default binder suppresses binding errors and silently skips unrecognized properties — document this when code depends on strict binding
11+
- Type conversion failures during binding must produce clear error messages identifying the configuration key and expected type
12+
- `IOptionsMonitor<T>` change notifications must propagate correctly to all subscribers during reload without race conditions
13+
- `ValidateOnStart()` calls must be idempotent — must not accumulate duplicate validation registrations
14+
15+
## Source Generator Parity (D13)
16+
17+
- Source-generated binding code must produce identical behavior to the runtime reflection-based binder for all supported types
18+
- Generated switch blocks over configuration keys must use case-insensitive comparison
19+
- Generated code must handle nullable types, default values, collections, and nested objects correctly
20+
- Incremental generators must handle cancellation and produce correct output on incremental changes
21+
- Test both source-generated and runtime code paths — parity failures are critical bugs
22+
23+
## Resource Ownership & Change Tokens (D4)
24+
25+
- File-based configuration providers must handle concurrent file replacement — use `FileShare.Delete` when reading
26+
- `IConfigurationRoot.Dispose` must dispose owned providers; document ownership for custom providers
27+
- Reload tokens must not leak event handler registrations — unsubscribe on disposal
28+
- Configuration providers own their data — `IConfigurationRoot` does not own `IFileProvider` instances
29+
30+
## Error Handling (D9)
31+
32+
- Exceptions must be the most specific applicable type with actionable context (key name, expected vs actual type)
33+
- Exceptions from inner operations must be wrapped or propagated — never swallowed silently
34+
- Configuration reload failures must be observable through logging or change notification callbacks
35+
36+
## Null Safety (D3)
37+
38+
- Input from configuration values and deserialized data must be validated for both null and semantic correctness
39+
40+
## Trim & AOT (D14)
41+
42+
- Annotate reflection-using binding APIs with `[DynamicallyAccessedMembers]` to preserve required metadata
43+
- Provide feature switches so the linker can trim optional binding functionality
44+
- No new IL2xxx trim warnings without explicit justification
45+
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
applyTo: "src/libraries/Microsoft.Extensions.DependencyInjection*/**"
3+
---
4+
5+
# Microsoft.Extensions.DependencyInjection — Folder-Specific Guidance
6+
7+
## Service Lifetime Correctness (D7)
8+
9+
- Use `TryAdd{Lifetime}` instead of `Add{Lifetime}` for default registrations to avoid overriding user-configured services
10+
- Scoped services must never be injected into singleton services (captive dependency) — the scoped service would live for the application lifetime
11+
- Transient IDisposable services are tracked by the container until scope disposal — prefer scoped lifetime for disposable services
12+
- Service registration order matters — later registrations of the same type override earlier ones unless `TryAdd` is used
13+
- Decorator patterns where inner and outer service share the same interface must guard against infinite recursion during resolution
14+
- Do not inject `IServiceProvider` broadly as a service locator — inject the specific service needed
15+
16+
## Service Resolution
17+
18+
- `ActivatorUtilities.CreateInstance` bypasses the container registration or factory for the type being created, but it still resolves constructor dependencies through the provided `IServiceProvider`, which can invoke factories for those dependencies
19+
- Singleton dependencies resolved lazily at first request must not cause response-time spikes — consider `ValidateOnStart`
20+
- Factory delegates (`Func<IServiceProvider, T>`) should follow established parameter ordering conventions
21+
22+
## Abstractions Package (D17)
23+
24+
- `IServiceCollection`, `IServiceProvider`, and related interfaces belong in `Microsoft.Extensions.DependencyInjection.Abstractions`
25+
- The concrete `ServiceProvider` and resolution engine belong in the implementation package
26+
- Moving types between abstractions and implementation is a breaking change
27+
- Package references must target correct and aligned versions across the dependency graph
28+
29+
## Thread Safety (D6)
30+
31+
- Singleton services are accessed concurrently by default — all singleton implementations must be thread-safe
32+
33+
## Trim & AOT (D14)
34+
35+
- Annotate service resolution that uses reflection with `[DynamicallyAccessedMembers]`
36+
- Ensure the DI container works correctly under NativeAOT — test with `PublishAot=true`
37+
- No new IL2xxx trim warnings without explicit justification
38+
39+
## Test Coverage (D10)
40+
41+
- Every bug fix includes a regression test; every new feature has happy-path and edge/error tests
42+
- Dispose behavior must be tested — verify resources are released and post-disposal operations throw `ObjectDisposedException`
43+
- Test captive dependency detection and resolution order across different registration patterns
44+
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
applyTo: "src/libraries/Microsoft.Extensions.Hosting*/**"
3+
---
4+
5+
# Microsoft.Extensions.Hosting — Folder-Specific Guidance
6+
7+
## Host & Service Lifecycle (D15)
8+
9+
- `Host.StopAsync` must not throw when the cancellation token is canceled — it must allow hosted services and host lifetime to complete their shutdown logic
10+
- Shutdown signal handlers (`UseConsoleLifetime`, SIGTERM) must correctly dispose registrations and propagate to all hosted services on all platforms
11+
- If an app exposes readiness health checks, it should avoid listening for or accepting work until its own readiness conditions are met
12+
- `BackgroundService.ExecuteTask` may be null if the derived class did not call `base.StartAsync` — never assume it is set
13+
- `BackgroundService.ExecuteAsync` exceptions must be observed and logged — unobserved task exceptions silently crash the host
14+
- Graceful shutdown must handle `OperationCanceledException` from the stopping token without logging it as an error
15+
16+
## Hosted Service Ordering
17+
18+
- Hosted services start in registration order and stop in reverse order — document dependencies between services
19+
- Do not assume ordering across `IHostedService` implementations unless explicitly registered in sequence
20+
- Consider `ValidateOnStart` to catch configuration errors during startup instead of at first request
21+
22+
## Thread Safety & Static State (D6)
23+
24+
- Two hosts running in the same process must not interfere via static state
25+
- Static event handlers and registrations must be scoped to the host instance, not the process
26+
27+
## Cross-Platform Correctness (D19)
28+
29+
- Shutdown signal handling (SIGTERM, SIGINT, Ctrl+C) must work correctly on Windows, Linux, and macOS
30+
- Platform-specific behavior differences must be abstracted behind PAL layers or guarded with runtime checks
31+
32+
## Error Handling (D9)
33+
34+
- Exceptions from hosted services must be properly observed — unhandled exceptions in `ExecuteAsync` must not silently terminate the host
35+
36+
## Test Reliability (D11)
37+
38+
- Hosted service lifecycle tests must use generous timeouts (3+ minutes in stress/JIT stress pipelines)
39+
- Avoid tight timing assertions — prefer event-based synchronization over `Task.Delay`-based waits
40+
- Tests must not leak global state (environment variables, static fields) across test runs
41+
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
applyTo: "src/libraries/Microsoft.Extensions.Logging*/**"
3+
---
4+
5+
# Microsoft.Extensions.Logging — Folder-Specific Guidance
6+
7+
## LoggerMessage Source Generator (D16, D13)
8+
9+
- `[LoggerMessage]`-attributed methods must be `static partial` — non-static methods cause confusing source generator errors
10+
- Structured log messages must use template placeholders (`{Name}`) — never embed string interpolation, which bypasses structured logging
11+
- Log exceptions via the `ILogger` overload that accepts `Exception` as a parameter — do not embed `exception.ToString()` in the template
12+
- Source-generated logging must produce identical behavior to the runtime logging path — test both paths for parity
13+
- Generated code must compile without warnings, especially nullable warnings
14+
15+
## Log Levels & Security
16+
17+
- Never log sensitive data (credentials, tokens, PII, request/response bodies) at any level, including Debug/Trace
18+
- `IsEnabled` checks should guard expensive log message construction when not using source-generated methods
19+
20+
## Error Handling & Diagnostics (D9)
21+
22+
- Exceptions from inner operations must be properly wrapped or propagated — do not swallow silently without logging
23+
24+
## Logging Provider Configuration
25+
26+
- Console formatter behavior and scope inclusion must follow established defaults
27+
- Changes to provider configuration must not break existing consumer expectations for log output format
28+
- Logging scope semantics must propagate correctly across async boundaries
29+
30+
## Abstractions vs Implementation (D17)
31+
32+
- `ILogger`, `ILoggerFactory`, `ILoggerProvider`, and logging attributes belong in `Microsoft.Extensions.Logging.Abstractions`
33+
- Concrete providers (Console, Debug, EventSource) belong in their respective implementation packages
34+
- Do not introduce implementation dependencies in the abstractions package
35+
36+
## Performance (D5)
37+
38+
- Hot logging paths (per-request, per-operation) must use `[LoggerMessage]` source generation for zero-allocation logging
39+
- Avoid closures that capture state on frequently called log paths — use static lambdas with explicit state
40+
- String manipulation on log paths must use Span-based APIs to avoid substring allocations
41+

0 commit comments

Comments
 (0)