Skip to content
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

Implement script async execution control #246

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ixjf
Copy link
Contributor

@ixjf ixjf commented Feb 24, 2019

MoonSharp already has an extension for executing scripts asynchronously. However, that extension allows pretty much no control over the script's execution. Currently, the only way to do so is through coroutines, which involves writing a bunch of boilerplate every time, and which also makes it more difficult to implement, say, a C#-land binding that needs to pause the script thread (imagine a 'sleep' function). Coroutines also don't really allow scripts to be run in the background. They are still run in the foreground, but pause every once in a while. In order to actually run them in the background AND be able to control their execution, one has to add more control code (Lua thread running coroutine and checking every x instructions for abort, main thread creating that separate Lua thread and controlling it via a reset event).

Right now, we want to:

  1. be able to stop the script without forcing an abort
  2. be able to pause the script without a Thread.Sleep, which makes the script thread unresponsive to any kind of abort code
  3. run a Lua script in the background and continue with the normal flow of the program

Additional functionality could obviously be added, like pausing & then resuming execution from within the main flow of the program.

Having support for this in the library makes sense considering it already has async methods. It is also better because it makes everyone else's code cleaner and requires less boilerplate to be written.

This is implemented as follows:

namespace MoonSharp.Interpreter
{
    public class ExecutionControlToken
    {
        public ExecutionControlToken();

        public void Terminate();

        // ...
    }
}

'ExecutionControlToken' provides control of the execution of a script.

Calling 'Terminate' will raise a ScriptTerminationRequestedException from the thread that is running the Lua script. All exceptions can be caught in the same way as with the existing async methods.

namespace MoonSharp.Interpreter
{
    public class Script
    {
        // ...

        public Task<DynValue> DoStringAsync(ExecutionControlToken ecToken, string code, ...);

        public Task<DynValue> DoStreamAsync(ExecutionControlToken ecToken, ...);

        public Task<DynValue> DoFileAsync(ExecutionControlToken ecToken, ...);

        // ... other existing async methods

        public Task<DynValue> CallAsync(ExecutionControlToken ecToken, ...);

        // ...
    }
}

The first three methods are modified from the original MoonSharp. They have an additional parameter in the 1st position, which is an 'ExecutionControlToken'. This 'ExecutionControlToken' becomes associated with the execution of the code specified, and it can be associated with multiple scripts.

... is parameters from the non-async methods.

Because 'ecToken' is added as a first parameter to these async methods, this will break compatibility with current 2.x version.

namespace MoonSharp.Interpreter
{
    public class ScriptExecutionContext
    {
        // ...
    
        public void PauseExecution(TimeSpan timeSpan);
    
        // ...
    }
}

'PauseExecution' is added to 'ScriptExecutionContext' so that C#-land bindings can pause the script thread. This function responds to abort requests, so any call to 'PauseExecution' won't block the normal flow of the program.

Although async extensions are only supported on .NET 4.0+, 'PauseExecution' works on .NET 3.5 as well. On that platform, it simply calls Thread.Sleep. That is because there is no async support anyway, so the script execution is already blocking the thread. This is simply for uniformity.

Update as of 18/05/2019: I still have some doubts regarding this pull request, particularly about how the following:
- I have to pass the token around everywhere in internal code.
- Pausing execution of a script from outside a CLR function binding did not seem useful to me at the time I did this. I'm now reconsidering.
- Calling Script.Load/LoadAsync on the same Script instance being used by a script running asynchronously might fail since there is an attempt to access the processor from multiple threads, meaning loading code has to be done before executing the script. I'm not completely sure this is a problem, haven't looked at the code in a while, but probably is.

@blakepell
Copy link

blakepell commented May 18, 2019

@ixjf

I was checking this pull request because it looks exactly what I'm looking for in terms of being able to temporarily pause a Lua script.

I do have one question on the implementation though. The ExecutionControlToken was added as the first parameter which as you stated breaks backwards compatibility. What's the downside from putting it at the end and having an overload or optional parameter with a default value so that old calls continue to work alongside the new ones?

@ixjf
Copy link
Contributor Author

ixjf commented May 18, 2019

It's not possible because some of the async functions take a variable number of arguments at the end, unfortunately.

@ixjf
Copy link
Contributor Author

ixjf commented May 18, 2019

I can't think of an alternative which doesn't break compatibility, and leaving the existing functions and adding overloads doesn't make sense to me.

@ixjf
Copy link
Contributor Author

ixjf commented May 18, 2019

I have to note that when associating the same token with multiple scripts, calling PauseExecution from the ScriptExecutionContext in a CLR function binding will stop all of them which are associated. Not sure if this should be the behaviour, but it currently is in order to be able to call PauseExecution on the token from outside those script threads.

@ixjf
Copy link
Contributor Author

ixjf commented May 18, 2019

Also, at the time I did this I did not think it could be useful to be able to call PauseExecution from outside CLR function bindings, though now I'm reconsidering.

@xanathar
Copy link
Member

Whoa this is big.

Give me a little more time, sorry.

Thanks for the contribs!!

@lofcz
Copy link

lofcz commented Mar 29, 2021

@xanathar what's the state of this? I've tried the branch and lgtm.

@ixjf
Copy link
Contributor Author

ixjf commented Mar 29, 2021

I haven't worked on this for a long time, but besides the doubts I have that I have already presented above, I also need to clean up the pull request. There seem to be a lot of project file changes that probably aren't needed and just happened as I went about modifying the original code.

@xanathar
Copy link
Member

I feel ashamed for the response time ... sorry 😳 (and thanks lofcz for the tag, otherwise I'd have missed this again).
I don't think there's plenty of usage for the current async methods as they are quite poor in functionality, so I'm quite fine with breaking that compatibility. It also seems to me that users of the current async API (if any exist) can on-board on the new one with just a trivial change, so, I'd go on.
For me it's fine to merge on main as soon as the project files are cleaned up from accidental changes (intentional ones are of course fine).

Thanks for the amazing patch.

@ixjf
Copy link
Contributor Author

ixjf commented Mar 31, 2021

Great, thanks :) I think I will need another month before I can clean this up, though, since I'm a little busy with university right now.

@blakepell
Copy link

@ixjf Thought I'd share, I've been using your changes for a year now on a personal project and they've worked well for me. Thanks for sharing, look forward to seeing the new pull request.

@LimpingNinja
Copy link
Contributor

@ixjf Just wanted to check in on this, if you get it clean I will merge it.

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

I think I got it as clean as it can be. Still a pretty huge commit, though.

@lofcz
Copy link

lofcz commented Nov 28, 2021

@ixjf thanks for cleaning this up! Looking trough the tests and wondering how to wire up async Task<T> F(ScriptExecutionContext ctx, CallbackArguments args) with this (eg. a delegate of type (Func<ScriptExecutionContext, CallbackArguments, Task<T>>). Is this possible?

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

Why do you need the function to return the task? You get a Task when you do CallXAsync.

@lofcz
Copy link

lofcz commented Nov 28, 2021

I'm fetching data from DB and I'd like to free the thread while that is being done to avoid thread starvation. In my use case moonsharp scripts are being used as backend for a web application.

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

So what you want is to be able to pause the task indefinitely when you ask for data from the DB and be able to trigger it to resume when the data is ready?

@lofcz
Copy link

lofcz commented Nov 28, 2021

Essentially I just need to make the whole chain of function calls up to the one fetching data from DB async and awaiting each other. When this function is called the thread can handle other requests rather that waiting (being blocked) for the data to arrive.

moonsharp: -> [implicit await] db.query("...") -> [await] c# method "query" is called -> [current thread is now free to do whatever is needed, we are waiting for the data] -> data arrives -> "query" in c# finishes -> "query" in moonsharp finishes -> script execution continues

Just like callbacks but with current thread able to do other work while callbacks are resolving.

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

Can you give me some sample code of both Lua and C# land? Easier to visualize what you're trying to do.

@lofcz
Copy link

lofcz commented Nov 28, 2021

C#:

Script script = new Script();
script.Globals["dbfn"] = (Func<ScriptExecutionContext, CallbackArguments, Task<int>>)DbFn;

async Task<int> DbFn(ScriptExecutionContext ctx, CallbackArguments args) {
   return await (int)dbcontext.QueryAsync($"select top 1 someIntColumn from Users where id = {(int)args[0].Double}"); // whatever
}

Lua:

n1 = dbfn(1);
n2 = dbfn(2);
n = n1 + n2; -- do whatever with n1,n2 here...

related to: #228

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

When you call dbfn, do you expect the Lua code to continue on to the next dbfn call? Or what?

@LimpingNinja
Copy link
Contributor

Hey everyone, at this point maybe it is better to either bring it to the Discord or bring it to Github Discussions for this repo?

@lofcz
Copy link

lofcz commented Nov 28, 2021

@ixjf I need/expect Lua to pause execution until the value is resolved. On the second line I need to be able to work with n1 already.

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

Right, you want it to block, just not keep the Lua thread running while waiting for the results from the QueryAsync thread. Got it.

@LimpingNinja I was thinking that I might be able to add this one functionality (if possible) to this pull request. But maybe this is big enough a change already?

@LimpingNinja
Copy link
Contributor

@ixjf Let's spawn this off to a different discussion, I need to review this one in context and apply it then we can look at iterative adjustments like this request.

@ixjf
Copy link
Contributor Author

ixjf commented Nov 28, 2021

Fair enough.

@lofcz
Copy link

lofcz commented Nov 28, 2021

Thanks for considering this! Would be a game changer for me :)

@Arakade
Copy link

Arakade commented Dec 14, 2021

Right, you want it to block, just not keep the Lua thread running while waiting for the results from the QueryAsync thread. Got it.

@LimpingNinja I was thinking that I might be able to add this one functionality (if possible) to this pull request. But maybe this is big enough a change already?

Where can we follow development of the new async work mentioned here, please? @ixjf

I'm converting our WIP strategy game (which uses MoonSharp for moddev) to using C# async/await for its level loading, web requests etc. This has been going fine but I just realised I need to be able to call async C# methods from within Lua code. The proposal above sounds 👨‍🍳 🤌 (french chef kiss) perfect! (I had expected I'd have to require shotgunning with coroutines or extra keywords like async and await.)

Thanks

@lofcz
Copy link

lofcz commented Dec 17, 2021

@Arakade exactly, we are in the same boat here 👍🏻

@lofcz
Copy link

lofcz commented Mar 25, 2022

There is now an implementation of the feature I've requested in comments - WattleScript/wattlescript@2c52d89 by @CallumDev
Might be a good inspiration for @ixjf in case you'd like to get it upstream.

cc: @Arakade

@lonverce
Copy link

lonverce commented Aug 16, 2023

Fortunately, I find a way to solve these problems above, @lofcz @ixjf

First of all, we should realize that Coroutine is the key point since it makes the lua script to be able to "pause" itself on its own, "sending" something to outside, and "waiting" for something back so that the script resume.

Assuming we have a CLR method which returns a Task:

public static async Task<string> ReadFileText( string fileName ){
    return await File.ReadAllTextAsync(fileName);
}

And I wish it can be invoked in lua like this below:

local txt = readFileText("C:\\test.txt");

In order to do that in MoonSharp, we have to "send" this Task object to CLR code by Coroutine, so that CLR code can help us to await this Task object, after Task complete, CLR code retrieves the Result from Task object and "send" it back to lua by Coroutine.

It's sound seems like this, basic version:

local txt = coroutine.yield( readFileText("C:\\test.txt") ); -- send the Task object to outside and wait for result.

The coroutine.yield is good, but I still DONT want to write it in my lua code. therefore, I "hide" it by using SetClrToScriptCustomConversion.
With this conversion, MoonSharp will automaticlly converts the Task object to a YieldRequest which means calling coroutine.yield( task ) in lua.

Step 1: Define a wrapper for Task and Task<T>

public class TaskDescriptor
{
    public Task<object?> Task { get; private set; }

    public bool HasResult { get; private set; }

    public static TaskDescriptor Build(Func<Task> taskAction)
    {
        return new TaskDescriptor
        {
            Task = System.Threading.Tasks.Task.Run(async () =>
            {
                await taskAction();
                return (object?) null;
            }),
            HasResult = false
        };
    }

    public static TaskDescriptor Build<T>(Func<Task<T>> taskAction)
    {
        return new TaskDescriptor
        {
            Task = System.Threading.Tasks.Task.Run(async () => (object?)await taskAction()),
            HasResult = true
        };
    }
}

Step 2: Set a conversion for TaskDescriptor

Script.GlobalOptions.CustomConverters.SetClrToScriptCustomConversion<TaskDescriptor>((script, task) =>
{
    // Important !!!
    return DynValue.NewYieldReq(new[]
    {
        DynValue.FromObject(script, new AnonWrapper<TaskDescriptor>(task))
    });
});

Step 3: Write a Demo class which exposes CLR method to Lua

public class Demo
{
    public static TaskDescriptor Delay(int seconds) {
        // wrapper the Task by TaskDescriptor
        return TaskDescriptor.Build(async () => await Task.Delay(seconds * 1000));
    }

    public static TaskDescriptor Read(string path)
    {
        // wrapper the Task<T> by TaskDescriptor
        return TaskDescriptor.Build(async () => await File.ReadAllTextAsync(path));
    }
}

Step 4: Write an extensions method for Script

public static class ScriptExtension
{
    public static async Task<DynValue> DoStringAsync(this Script script, string codeScript, CancellationToken cancellation = default)
    {
        cancellation.ThrowIfCancellationRequested();

        //  load the code without running
        var code = script.LoadString(codeScript);

        // create an coroutine for running the code
        var coRoutine = script.CreateCoroutine(code);

        coRoutine.Coroutine.AutoYieldCounter = 1000;

        DynValue scriptResult;
        DynValue? resumeArg = null;

        while (true)
        {
            scriptResult = resumeArg == null ? coRoutine.Coroutine.Resume() : coRoutine.Coroutine.Resume(resumeArg);

            resumeArg = null;

            if (scriptResult.Type == DataType.YieldRequest) // AutoYieldCounter 
            {
                cancellation.ThrowIfCancellationRequested();
            }
            else if (scriptResult.Type == DataType.UserData)
            {
                if (scriptResult.UserData.Descriptor.Type != typeof(AnonWrapper))
                {
                    break;
                }

                var userData = scriptResult.UserData.Object;

                if (userData is not AnonWrapper<TaskDescriptor> wrapper)
                {
                    break;
                }

                var taskDescriptor = wrapper.Value;

                var taskResult = await taskDescriptor.Task;

                if (taskDescriptor.HasResult)
                {
                    resumeArg = DynValue.FromObject(script, taskResult);
                }
            }
            else
            {
                break;
            }
        }

        return scriptResult;
    }
}

Step 5: Test in Main

static async Task Main(string[] args)
{
     UserData.RegisterType<Demo>();

    Script.GlobalOptions.CustomConverters.SetClrToScriptCustomConversion<TaskDescriptor>((script, task) =>
    {
        // Important !!!
        return DynValue.NewYieldReq(new[]
        {
            DynValue.FromObject(script, new AnonWrapper<TaskDescriptor>(task))
        });
    });

    try
    {
        var script = new Script();
        script.Globals["demo"] = typeof(Demo);

        var r = await script.DoStringAsync(await File.ReadAllTextAsync("TestScript.lua"));
        Console.WriteLine(r.ToPrintString());
    }
    catch (Exception e)
    {
        Console.WriteLine("The lua script abort with exception. \n{0}", e);
    } 
}

TestScript.lua

print("Hello World! This is Lua code script.");

print("Before delay...")
demo.delay(3);
print("After delay")

local content = demo.read("TestScript.lua");

print(content);

return 0;

@vector76
Copy link

Thank you @lonverce, I used this code and it worked perfectly for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants