-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add support for async/await #12
Comments
For what it's worth I no longer get an "invalid program" when I try to shim async methods. The program compiles and runs fine. The shim, however, is never invoked. I suspect this might be (directly) related to how the compiler translates async methods into state machines. Thus, the original method (as it was in code) does When stepping through the code, I can see that we actively rewrite the code for AsyncTaskMethodBuilder. This means that we at least get to that level which means that we should at least be able to also rewrite the intended method. Related resource: https://devblogs.microsoft.com/premier-developer/extending-the-async-methods-in-c/ I tried asking ChatGPT how best to solve this issue: https://chat.openai.com/share/6f30f7a9-18d6-4b68-9d20-3f557fb5d7dd |
I believe we need to rewrite the state machine at runtime. That is, the state machine that is emitted by the compiler. Currently, we cannot get to that point because we do not rewrite methods on types in System.Private.CoreLib. We would need to change that so that we allow rewriting methods on
Please note that I have only attempted this with After some more testing (see comment below) I believe we can make do with just rewriting the
Rewriting the state machine is necessary because we normally only rewrite the methods (and their calls) from the entry point. However, when we add async into the mix, the methods cannot be found in the call tree of the entry point. This is due to how async methods are compiled. That said, rewriting the state machine enables us to follow the call tree which results from the async method calls. Please note that AlternativeAn alternative to the above would be to provide our own implementation of If we do this, it should be enough to rewrite the method in |
I got a minor breakthrough today. I've been able to intercept the call if I supply my own instance of There are, however, still a lot of intricacies I need to iron out. For instance, the method won't return. |
Update on the findings from here: #12 (comment) When I attempt to rewrite the method I am unable to properly execute the (resulting) program. At this point, I think I need to go back to the basics; I need to get the IL for the compiled assembly and then rewrite the necessary parts manually by hand to get a feeling for the IL code. Only then can I know what IL to emit. Also mentioned in the previous comment was the need for shimming the |
Following is the IL for the Sandbox app: Sandbox.txt What we want to do now is inject just enough IL to:
The next concern is what we should do with the remaining instructions. Should we still emit them? Does it make sense? |
During my attempts to rewrite the state machine I've run into the issue that the rewrites complains that the However, I think it could be fixed by simply emitting the call as-is without attempting to rewrite it. After all, the I believe we can add support for async methods. What we need to recognize is that the original async method does not exist in the compiled code. What exists instead is a state machine emitted by the compiler. This state machine implements
To support async methods we need to recognize this fact. This means that we should not look for the method by the name given in the expression to Assuming that we come up with a way to correctly recognize the We can fix this in the following ways:
I think option 1 is feasible. We could locate the label by going backwards through the instructions. Of course, this is complicated by the fact that we still need to honor all the states of the emitted state machine. In other words, if the original method contains two I see the following solutions to this:
Note that for this to work we cannot reuse the state numbers from the original method i.e. we need to use state numbers at least one higher than the highest state number in the original method. Addendum: Come to think of it, the state machine can be found by reflection by inspecting the nested types of the type containing the async method. (Example follows below.) The name of the state machine will always be the (full?) name of the method suffixed by some characters which makes the name an invalid type name in C# (the name is only valid in IL). We could maybe get the type of the state machine this way when the user calls Example: The following class: public class Program
{
public static async Task<int> GetIntAsync() => await Task.FromResult(1);
} Will be compiled to something like the following: public class Program
{
public static async Task<int> GetIntAsync() => // Code to invoke state machine below
internal class GetIntAsync()d__2 : IAsyncStateMachine
{
// Implementation of state machine among which we find:
void MoveNext()
{
// Code for async machinery and somewhere down the line...
Task.FromResult(1);
// Code to set result on the state machine
}
}
} Please note that I'm not entirely sure about the names and access modifiers (I'm writing this from my phone). |
Resource for the ExecutionEngine exceptions I am sometimes seeing: https://robbelroot.de/blog/fixing-the-system-executionengineexception/#What-is-an-ExecutionEngineException Additional resources: |
Hi, I can help with the testing, I have a lot of cases, which I can mock up. |
@asukachev You're not going to believe it! Support for async methods is here! Please, please, please, check out the code locally and run it through its paces on your machine! The tests target the following frameworks:
|
It turns out that the solution was simply to devirtualize the method differently if and only if the method is an async method. Normally, we can get the method simply by calling Whatever the reason, we can find it by going through the An added bonus is that the replacement methods will have their own state machines already generated by the compiler. Therefore, at that point we're really just moving the pointer from original state machine to the replacement state machine. This gives us the best solution I could hope for as:
RationaleThe reason that it async methods work is that we now actively rewrite the Resolving the |
@asukachev Try this: public async static Task AsyncContext(String st)
{
Console.WriteLine("Started StaticMethod()");
int value = st.GetCount();
Console.WriteLine("Value produced: " + value);
await Task.Delay(1000);
Console.WriteLine("Completed StaticMethod()");
}
static void Main()
{
Shim extensionShim = Shim.Replace(() => TestExtensions.GetCount(Is.A<string>())).With(delegate (string str) { return 2; });
PoseContext.Isolate(() =>
{
String str = "qwerwq";
AsyncContext(str).GetAwaiter().GetResult();
}, extensionShim);
Console.WriteLine("Finished");
} Ref: #27 (comment) |
I assume it must work, but it didn't for me.
|
@asukachev I will look into it later today. Thank you for providing your environment. UPDATE 2024-02-05: I was able to get everything working with the following two changes:
That leads me to believe there are the following issues we must address:
It seems that async methods are compiled (or at least started) differently in .NET 7. I've compiled a diff (link: https://www.diffchecker.com/fLB4MLdp/) of the output from running the code below against .NET Core 2.0 and .NET 7. Have a look at line 132 (image below). Perhaps this issue is present in targets lower than .NET 7. Will need to investigate further. public async static Task<int> AsyncContext(string st)
{
Console.WriteLine("Started StaticMethod()");
int value = st.GetCount();
Console.WriteLine("Value produced: " + value);
await Task.Delay(1000);
Console.WriteLine("Completed StaticMethod()");
return 1;
}
public static void Main(string[] args)
{
Shim extensionShim = Shim
.Replace(() => TestExtensions.GetCount(Is.A<string>()))
.With(delegate (string str) { return 2; });
PoseContext.Isolate( () =>
{
var x = AsyncContext("abc").GetAwaiter().GetResult();
}, extensionShim);
Console.WriteLine("Finished");
} UPDATE 2024-03-05: I've made some progress and discoveries related to this. Please see comments: #12 (comment) and #12 (comment) @asukachev Could I ask you to try the steps described in the Workaround section in #12 (comment)? |
As mentioned in the above comment (#12 (comment)) the following two changes were needed to make the code work as expected:
In the following I am using the shim below. Shim
.Replace(() => TestExtensions.GetCount(Is.A<string>()))
.With(delegate (string str) { return 2; }); Order of operationsCall before Task.DelayWith the following order of operations: public async static Task<int> AsyncContext(string st)
{
Console.WriteLine("Started StaticMethod()");
int value = st.GetCount();
Console.WriteLine("Value produced: " + value);
await Task.Delay(1000);
Console.WriteLine("Completed StaticMethod()");
return 1;
} I get the following output;
AnalysisNotice that Started StaticMethod() occurs twice, and that Value produced: also occurs twice although with different results each time. Task.Delay before callHowever, with the following order of operations ( public async static Task<int> AsyncContext(string st)
{
Console.WriteLine("Started StaticMethod()");
await Task.Delay(1000);
int value = st.GetCount();
Console.WriteLine("Value produced: " + value);
Console.WriteLine("Completed StaticMethod()");
return 1;
} I get the following output;
AnalysisNotice that Started StaticMethod() occurs twice, and that Value produced: also just once as it should and it has the expected result (due to the shim being invoked). 2 x
|
Following is the IL emitted by the method rewriter:
AnalysisLet's get the obvious of out the way:
I will compare files "implementation" and "real". The difference in length seems to be caused by the following three instructions:
As of right now I cannot conclude anything with regards to them not being present in the implementation. Perhaps they are needed; perhaps not. Debugging my way through I will proceed to step my way through Alas. Please enjoy the following from the predictive debugger in Rider: A grayed out line means that it will not be executed. Also note the comment regarding emitting the leave instruction if and only if it's NOT being used in an exception block. This is why those three instructions are left out. Let's see what would happen if we were to emit them anyway. With those three I am now experiencing something very weird: When I execute the code normally (without a debugger attached), the shim is not called. However, when the debugger is attached (and I explicitly break in the |
Notes to self for the adventure on another day:
AdventureIf Let's compare the compiler emitted IL against our emitted IL. Following are the updated IL files:
It seems that the implementation now has two
Leaving those out (via debugging and manually moving the stack pointer to not emit the instruction) leaves me with IL which has the expeced number of Below I've highlighted just the
Some notes:
What is interesting here is that the 5th instruction jumps to a different label than the expected one. In fact, the label it jumps to will update the state of the state machine and only then return. I'm having a hard time seeing this being the issue. UPDATE 2024-02-12: I can consistently reproduce the issue by attempting to shim anything from within an async method. The peculiar thing is that if the As mentioned above the very last |
Just to be clear: shimming async methods should work if the core logic of the replacement comes before the first I know that's far from ideal. I just wanted to get it out there while I'm continuing to work on thorough support for shimming async methods. |
I tried trimming the example to reach a minimum example. I came up with the following: await Task.Delay(1);
var value = TestExtensions.GetCount(st);
Console.WriteLine("Value produced: " + value);
return 1; This outputs:
Now, look at the example below. await Task.CompletedTask; // This task is already complete
var value = TestExtensions.GetCount(st);
Console.WriteLine("Value produced: " + value);
return 1; The example above works and outputs:
I suspect this might be due to the task already being completed. In this case, the state machine does not halt; it simply continues to the next line of code. This may be a hint that the issue lies elsewhere. Perhaps we need to up one level. I will need to inspect the IL where the state machine is started. Maybe the issue is with how control is yielded back to the callee. Comparing the IL for the two above examples, I cannot see any difference (aside from the call to
Additional filesIn this section I have used the following examples:
|
Follow upI am following up on the findings in #12 (comment) I will attempt to set a breakpoint in Given the following excerpt from the emitted IL (full file below the example) we know:
That means that the issue lies somewhere in those ~40 lines. We can narrow this further by noting that:
That means that the fault has to be somewhere between label IL_0041 and IL_008c.
Full IL: mve-tracing-test-emitted.txt It would be interesting to see which state machine we are executing before and after the It works?For more context please see the following comment: #12 (comment) Next steps are to investigate the significance of having a breakpoint after the first |
Adding support for .NET Core 2.1+In completely other news, I tried adding support for .NET Core 2.1 and greater. I was successful. It turns out that from .NET Core 2.1 and later, the async machinery forwards to I added an exception specifically for This seems to have added support for shimming async methods. What's more is that it looks like it works across public static class TestExtensions
{
public static int GetCount(this string s)
{
return 23;
}
}
public async static Task<int> Test()
{
Console.WriteLine("123".GetCount());
await Task.Delay(1);
Console.WriteLine("1234".GetCount());
return 1;
}
public static void Main(string[] args)
{
Shim extensionShim = Shim
.Replace(() => TestExtensions.GetCount(Is.A<string>()))
.With((string str) => 2);
PoseContext.Isolate( () =>
{
Test().GetAwaiter().GetResult();
}, extensionShim);
Console.WriteLine("Finished");
} I get the following output:
CaveatThe caveats are that:
The breakpoint is hit twice:
Further investigation shows that it's really only the second hit which is significant. Below are the variables for the 1. When rewriting
|
The significance of the breakpointI will attempt to inspect the stack before and after the I will be printing the stack trace using the following code: var stackTrace = new StackTrace();
foreach (var stackFrame in stackTrace.GetFrames())
{
Console.WriteLine(stackFrame);
} With breakpointsAs indicated in the Workaround section in #12 (comment) there is a difference between running with a breakpoint and running without a breakpoint. In the following, the breakpoint is set as indicated by step 3 in the aforementioned section. This gives me the following stack traces:
AnalysisNOTE: Methods starting with "stub_" and "impl_" are rewritten methods. There is a single difference between the stack traces. Namely, that the "after" stack trace contains the
Disregard the above difference, we are left with two stack traces which look very similar. However, then we notice that the "before" stack trace contains 2 calls to
Without breakpointsIn the following I will be running the code without breakpoints. That is, simply executing the code. This gives me the following stack traces: AnalysisThe two stack traces are wildly different. The "after" stack trace is a mere 3 lines. Specifically:
We notice that none of the methods in the above stack trace start with "stub_" or "impl_", meaning that they are not our (rewritten) methods but the original non-rewritten methods. With breakpoint vs. without breakpointIn the following I will compare the "before" stack trace when running with breakpoints and running without breakpoints. The major difference is that the stack trace with breakpoints enabled contains 2 calls to
What's more interesting is that the calls seem to have the following order:
In other words, the Additional conundrumI tried rerunning the code without breakpoints and encountered the following alternative stack trace:
This stack trace is very different from the one I'm normally seeing which is this:
Maybe this is nothing. I would hazard a guess that we are simply seeing the inner workings of the async machinery. |
@Miista I've tried Poser 2.1.0-alpha0001 and found some limitations which I confirmed using the Sandbox application from branch Doing the following:
Resulted in:
It turns out that shimming a static method used by an async method doesn't work at all ! This is sad since this was the use-case I had for Poser. |
Hi @fred-gagnon This is exactly the issue I've been battling in this issue. See #12 (comment) The issue is caused by there being an It seems that when there is an await in the async method, any code after the first await is run as-is and calls are not replaced with shims. UPDATE: So after some investigation I found that it's only the part up to the first I suspect this might have something to with how the state machine schedules its callback. That is, when the state machine suspends, it calls |
Plan:
Investigate what the IL should look likeImplement the translationPlease see Add support for async/await #12 (comment)Before we start working on this issue, though, I want to get the solution working first.
Related:
Blocked by:
The text was updated successfully, but these errors were encountered: