-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Don't set breakpoints in async thunk methods #123051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
+40,386
−4,563,547
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…net#122986) # Description The error message "DOTNET_DbgEnableMiniDump is set and the createdump binary does not exist" was being printed during runtime initialization, even when no dump would be created. This prevented setting the environment variable in crossgen scenarios where createdump isn't shipped. **Changes:** - Removed file existence check (`stat()`) during initialization in `PalCreateDumpInitialize()` to avoid unnecessary file system operations - Detect missing createdump binary when `execv()` returns `ENOENT` error in `CreateCrashDump()` - Print specific error message only when actually attempting to execute the missing binary - Error now appears only when dump creation is attempted (either via crash or explicit API call), not during initialization # Customer Impact Customers using crossgen tooling cannot set `DOTNET_DbgEnableMiniDump` without spurious error messages during builds. This affects debugging workflows in environments where dump generation is conditionally needed. # Regression No. This is an existing issue with the environment variable handling. # Testing Built NativeAOT runtime successfully with changes. Verified: - No initialization errors when createdump missing - Error message appears when attempting to execute missing createdump binary - No change in behavior when createdump exists - No unnecessary file system checks during initialization # Risk Low. Changes eliminate unnecessary file system checks during initialization and defer error detection to the point where the binary is actually needed (execv call). The actual dump creation logic and all success paths remain unchanged. Error messages now appear only when dumps are actually attempted, preventing spurious warnings during initialization. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Hard to use `DOTNET_DbgEnableMiniDump` when crossgen is involved somewhere in the build</issue_title> > <issue_description>https://github.com/dotnet/runtime/blob/242f7b23752599f22157268de41fee91cb97ef6c/src/coreclr/nativeaot/Runtime/unix/PalCreateDump.cpp#L627 > > This line runs ahead-of-time in the non failure case when setting the `DOTNET_DbgEnableMiniDump ` env var. The Crossgen2 pack currently doesn't ship createdump and therefore this line always raises an error when the env var is set. > > This isn't great as it means that you can't set `DOTNET_DbgEnableMiniDump` if crossgen is involved. I.e. when building the runtime repo. > > cc @hoyosjs </issue_description> > > <agent_instructions>Instead of having fprintf(stderr, "DOTNET_DbgEnableMiniDump is set and the createdump binary does not exist: %s\n", program); in PalCreateDumpInitialize in PalCreateDump.cpp, we should instead output it in BuildCreateDumpCommandLine inside the if (g_szCreateDumpPath == nullptr || g_ppidarg == nullptr) block</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes dotnet#122982 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: steveisok <471438+steveisok@users.noreply.github.com> Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com>
…uild (dotnet#123067) # Description The coding agent was skipping baseline setup steps before building/running tests, causing "missing testhost" and "shared framework" errors. Restructured instructions to make baseline build impossible to miss. **Key changes:** - Moved baseline build to **first section** after title with⚠️ MANDATORY heading - Added explicit cause-effect: skipping causes testhost/shared framework errors - Replaced verbose bullet lists with **quick-scan tables** for component ID and build commands - Added gate condition: "Only proceed after baseline build succeeds" - Reduced document from ~325 to ~185 lines (~43% reduction) - Removed table of contents and verbose iterative strategy section (standard behavior, not actionable) - Consolidated troubleshooting and references into compact table format **Restored guidance per feedback:** - WASM/WASI library detection logic (condensed) - "Checkout main branch" instruction before baseline build - Instruction to switch back to working branch after baseline build - Success criteria (exit code 0, zero test failures) - Library test project structure guidance with discovery command - "Target does not exist" troubleshooting entry - Log collection guidance for failure reporting **Added new components per feedback:** - Added entries for `src/tools`, `src/native/managed`, `src/tasks`, `src/tests` to component identification table - Added baseline build commands for Tools, Build Tasks, and Runtime Tests - Added workflow sections for Tools, Build Tasks, and Runtime Tests with build/test commands # Customer Impact N/A - Internal tooling instructions only. # Regression No. # Testing Documentation-only change. Verified no trailing whitespace per markdown guidelines. # Risk Low. No code changes. Core guidance retained; only organization and emphasis changed. # Package authoring no longer needed in .NET 9 IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. <!-- START COPILOT CODING AGENT SUFFIX --> <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > Our instructions aren't working as well as we'd like. In particular, the coding agent isn't doing the baseline setup steps before it tries building/running tests. Please make whatever changes are needed to make these instructions more effectove. That could include anything from minor tweaks to an overhaul. Please retain the core instructions and guidance; focus on organization, brevity, relevance to copilot, etc </details> <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
This pull request updates the following dependencies [marker]: <> (Begin:0c5a34f5-504e-413b-9376-08d8d8ff2d75) ## From https://github.com/dotnet/runtime-assets - **Subscription**: [0c5a34f5-504e-413b-9376-08d8d8ff2d75](https://maestro.dot.net/subscriptions?search=0c5a34f5-504e-413b-9376-08d8d8ff2d75) - **Build**: [20260109.1](https://dev.azure.com/dnceng/internal/_build/results?buildId=2875752) ([296684](https://maestro.dot.net/channel/8297/github:dotnet:runtime-assets/build/296684)) - **Date Produced**: January 9, 2026 9:10:15 PM UTC - **Commit**: [fd342f922435b2725113b5226c7e25fcf830a149](dotnet/runtime-assets@fd342f9) - **Branch**: [main](https://github.com/dotnet/runtime-assets/tree/main) [DependencyUpdate]: <> (Begin) - **Dependency Updates**: - From [11.0.0-beta.25626.1 to 11.0.0-beta.26059.1][1] - Microsoft.DotNet.CilStrip.Sources - Microsoft.DotnetFuzzing.TestData - Microsoft.NET.HostModel.TestData - System.ComponentModel.TypeConverter.TestData - System.Data.Common.TestData - System.Drawing.Common.TestData - System.Formats.Tar.TestData - System.IO.Compression.TestData - System.IO.Packaging.TestData - System.Net.TestData - System.Private.Runtime.UnicodeData - System.Runtime.Numerics.TestData - System.Runtime.TimeZoneData - System.Security.Cryptography.X509Certificates.TestData - System.Text.RegularExpressions.TestData - System.Windows.Extensions.TestData [1]: dotnet/runtime-assets@f256997...fd342f9 [DependencyUpdate]: <> (End) [marker]: <> (End:0c5a34f5-504e-413b-9376-08d8d8ff2d75) Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…0640) - [ ] Identify the failing test issue - [x] Fix XML string formatting to use double quotes instead of single quotes - [ ] Verify the test passes <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). Co-authored-by: Stephen Toub <stoub@microsoft.com>
… Interval on disabled one-shot timer (dotnet#122843) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
…tIListWhereIterator (dotnet#122729) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Update comment in KeyAnalyzer to reflect change from `IsAllAscii` to `Ascii.IsValid` in dotnet#119673
…ty (dotnet#122952) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
…porary allocations (dotnet#122038)
… object-converter that handles string (dotnet#122835) main PR # Description When a `CastingConverter<T>` wraps a source converter whose `Type` doesn't match `T` (e.g., `CastingConverter<string>` wrapping `JsonConverter<object>`), property name serialization causes infinite recursion: 1. `CastingConverter<string>.WriteAsPropertyNameCore` → source converter 2. Source's base `WriteAsPropertyName` gets fallback for type `object` → `ObjectConverter` 3. `ObjectConverter` looks up converter for runtime type `string` → returns same `CastingConverter<string>` 4. Loop → StackOverflow ```csharp // Reproducer public class GenericJsonConverter : JsonConverter<object> { public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(string); public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => reader.GetString(); public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteStringValue(value.ToString()); } var options = new JsonSerializerOptions { Converters = { new GenericJsonConverter() } }; var value = new Dictionary<string, int> { { "key", 123 } }; JsonSerializer.Serialize(value, options); // StackOverflowException ``` **Fix**: In `CastingConverter`, check if `_sourceConverter.Type == typeof(T)` before delegating property name methods. If they differ, use base class logic which correctly resolves fallback for type `T`. # Customer Impact Users with custom `JsonConverter<object>` converters that handle specific types via `CanConvert` will hit StackOverflowException when serializing dictionaries with keys of those types. # Regression No - this is a pre-existing issue. # Testing - All 49,801 System.Text.Json tests pass - Added regression test `ObjectConverterHandlingStrings_DictionaryWithStringKey_DoesNotCauseStackOverflow` with a test converter that writes distinctive values (empty string for `WriteAsPropertyName`, constant for `ReadAsPropertyName`) to verify the fallback converter is correctly used instead of the custom converter # Risk Low. The fix adds a type equality check before delegation. If types match, behavior is unchanged. If they differ, we use the base class fallback path which is already well-tested. # Package authoring no longer needed in .NET 9 IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>JsonSerializer.Serialize throws StackOverflowException serializing dictionary with string key and object-converter that handles string</issue_title> > <issue_description>_This issue has been moved from [a ticket on Developer Community](https://developercommunity.visualstudio.com/t/JsonSerializerSerialize-throws-StackOve/10935999)._ > > --- > [severity:It's more difficult to complete my work] > Having a custom "generic" serializer that handles multiple types (including string) and then serializing a dictionary with string key makes the Serialize call to get stuck in a forever loop (with stack overflow exception as result). Perhaps a bit odd use case but I stumbled on it after converting from Newtonsoft to .NET so I just let you know in any case. My real use case is a bit more complicated as usual but this is what I could boil it down to: > > ```C# > using System.Text.Json; > using System.Text.Json.Serialization; > > namespace JsonSerializationTest > { > // example minimal "generic" converter > public class GenericJsonConverter : JsonConverter<object> // using string here works but that is not an option for my real usecase > { > public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(string); > public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => reader.GetString(); > public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteStringValue(value.ToString()); > } > > public class Tests > { > [Test] > public void DoesNotWork() > { > var options = new JsonSerializerOptions(); > options.Converters.Add(new GenericJsonConverter()); > var value = new Dictionary<string, int> { { "key", 123 } }; > Assert.That(() => JsonSerializer.Serialize(value, options), Throws.InstanceOf<StackOverflowException>()); > } > > [Test] > public void Works() > { > var options = new JsonSerializerOptions(); > var value = new Dictionary<string, int> { { "key", 123 } }; > Assert.DoesNotThrow(() => JsonSerializer.Serialize(value, options)); > } > } > } > ``` > > > --- > ### Original Comments > > #### Feedback Bot on 14/07/2025, 09:07 AM: > > <p>We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.</p> > </issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@eiriktsarpalis</author><body> > I can reproduce, it appears to be an issue with how dictionary key serialization interacts with converters whose generic type doesn't match that of the serialized type. You could work around the issue either by making the converter be of type string, a[dopt the `JsonConverterFactory` pattern](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to#sample-factory-pattern-converter) for generic converters, or manually override the `WriteAsPropertyName` method: > ```C# > public class GenericJsonConverter : JsonConverter<object> // using string here works but that is not an option for my real usecase > { > public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(string); > public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => reader.GetString(); > public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteStringValue(value.ToString()); > public override void WriteAsPropertyName(Utf8JsonWriter writer, [DisallowNull] object value, JsonSerializerOptions options) => writer.WritePropertyName(value.ToString()); > } > ```</body></comment_new> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes dotnet#118381 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
OpenSSL marked a few methods that could return const as requiring const. Add const qualifiers and casts where needed.
…lationToken) overloads to ZipArchiveEntry (dotnet#122032) This PR adds new overloads to `ZipArchiveEntry.Open()` and `ZipArchiveEntry.OpenAsync()` that accept a `FileAccess` parameter, allowing users to specify the desired access mode when opening an entry stream. When a `ZipArchive` is opened in `ZipArchiveMode.Update`, calling `ZipArchiveEntry.Open()` always returns a read-write stream by invoking `OpenInUpdateMode()`. This causes the entire entry to be decompressed into memory, even when the caller only intends to read the entry's contents. New APIs: ```c# public Stream Open(FileAccess access); public Task<Stream> OpenAsync(FileAccess access, CancellationToken cancellationToken = default); ``` Update Mode Details: - `FileAccess.Read`: Opens a read-only stream over the entry's compressed data without loading it into memory. Useful for streaming reads. - `FileAccess.Write`: Opens an empty writable stream, discarding any existing entry data. Semantically equivalent to "replace the entry content entirely" (like `FileMode.Create`. The stream is backed by a `MemoryStream` and content is written to the archive when disposed. - `FileAccess.ReadWrite`: Same as parameterless `Open()`/`OpenAsync()` - loads existing data into memory and returns a read/write/seekable stream. Closes dotnet#101243 Closes dotnet#1544
This is basically a revert of dotnet/corert#2117 and port of dotnet/coreclr#7895 instead. Now that RyuJIT implements the optimization for `Equals` (dotnet#122779), we only need `GetHashCode` and the `GetHashCode` optimization can be implemented the same way how it's implemented on CoreCLR VM. Cc @dotnet/ilc-contrib @EgorBo
This PR removes the `SORT_MARK_STACK` feature and related `rqsort1` function from the GC. These aren't used and if we need them, we can always bring them back from history. I don't know the full history here but I assume it's not enabled because the performance benefits weren't worth the sorting cost. Plus we have prefetch in the mark phase now which should help hide memory latency.
Co-authored-by: Milos Kotlar <kotlarmilos@gmail.com>
…stack arg sizes through our data structures where possible (dotnet#123016) A quick analysis of the current use of PInvokeArgIterator leads me to believe that we would use it to calculate a value we never actually used. This PR changes the implementation of the runtime to remove those uses which were tied to construction of function pointers from delegates where the delegate parameters were a structure which required marshalling. Should be a small code size win for Unix X64 platforms along with a negligible perf boost on all non-windows platforms. We still need this thing for COM scenarios on Windows so I haven't touched it there. This work is not removing the PInvokeArgIterator itself, as it appears to be necessary for the interpreter call stub generator (although it also needs to be fixed to be correct in those cases as well.)
…custom VARIANT types in libraries (dotnet#123060) Co-authored-by: Aaron R Robinson <arobins@microsoft.com>
…rtion (dotnet#121527) When RangeCheck inspects, say, "i + cns" tree, it tries to get the "assertion-based" range for `i` via its block's `bbAssertionIn`. It's too conservative and doesn't take into account assertions created before that `i` in the block (inter-block assertions). Let's see if we can cheaply just accumulate those by hands (@AndyAyersMS's idea). A simplified version of the repro is the following (thanks @BoyBaykiller for the repro): ```cs void Test(int[] arr, int i) { arr[i] = 0; // creates 'i >= 0 && i < arr.Length' assertion i++; // same block as ^ if (i < arr.Length) arr[i] = 0; } ``` Codegen diff: ```diff ; Method Benchmarks:Test(int[],int):this (FullOpts) G_M59621_IG01: sub rsp, 40 G_M59621_IG02: mov eax, dword ptr [rdx+0x08] cmp r8d, eax jae SHORT G_M59621_IG05 mov ecx, r8d xor r10d, r10d mov dword ptr [rdx+4*rcx+0x10], r10d inc r8d cmp eax, r8d jle SHORT G_M59621_IG04 G_M59621_IG03: - cmp r8d, eax - jae SHORT G_M59621_IG05 mov eax, r8d xor ecx, ecx mov dword ptr [rdx+4*rax+0x10], ecx G_M59621_IG04: add rsp, 40 ret G_M59621_IG05: call CORINFO_HELP_RNGCHKFAIL int3 -; Total bytes of code: 56 +; Total bytes of code: 51 ```
* Add the Wasm SP as the first arg for methods with managed calling convention * Add the PE pointer as last arg for methods with managed calling convention Pass the SP arg to calls, but not (yet) the PE pointer. Adjust things in the JIT that assumed LclNum 0 for instance methods was "this".
The interpreter compiler was missing support for getting field address via helper call that is used in EnC scenarios. This change adds such support for both static and instance fields. It makes all System.Runtime.Loader.Tests libraries tests pass when running interpreted.
…untime into async-breakpoints
…s attempting to patch the thunk which is marked as a 'stub' because it is recognized by the StubManager
…to async thunk. add AsyncThunkStubManager to handle case when attempting to call already JITted async thunk.
Member
|
Wrong commit base? |
Member
Author
Closing in favor of #123644 |
This was referenced Jan 27, 2026
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area-Diagnostics-coreclr
linkable-framework
Issues associated with delivering a linker friendly framework
runtime-async
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.