tonybaloney / CSnakes

https://tonybaloney.github.io/CSnakes/
MIT License
275 stars 21 forks source link

Error #291

Open psxbox opened 3 days ago

psxbox commented 3 days ago

Error in calling python method from Blazor app:

info: CSnakes.Runtime.IPythonEnvironment[0]
      Setting up Python environment from C:\Users\adham\.nuget\packages\python\3.12.6\tools using home of D:\Projects\C#\_EXPERIMENT\CSnakes\BlazorApp1\BlazorApp1\wwwroot\modules
Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Repeat 2 times:
--------------------------------
   at CSnakes.Runtime.CPython.CPythonAPI.Py_DecRefRaw(IntPtr)
--------------------------------
   at CSnakes.Runtime.Python.GIL+PyGilState.Dispose()
   at BlazorApp1.Components.Pages.Home+<Test>d__3.MoveNext()

Python file:

from typing import Tuple

def test(string: str) -> str:
    return string

def info() -> Tuple[str, str, str, str]:
    """
    Get information about this module

    Returns a tuple containing name, description, module type and version
    """

    return "Demo", "This is demo", "DataReader", "1.0.0"

def callback(cb_func) -> bool:
    value = cb_func()
    return value == "Hello, World!"

C# code:

private async Task Test((string fileName, (string name, string desription, string type, string version)) item)
    {
        try
        {
            using (GIL.Acquire())
            {
                var fileName = Path.GetFileNameWithoutExtension(item.fileName);
                using PyObject module = Import.ImportModule(fileName);
                using var infoFunc = module.GetAttr("test");
                var testStr = "Hello, World!";
                using var pyStr = PyObject.From(testStr);
                using var result = infoFunc.Call(pyStr);
                var info = result.As<string>();
                if (info != testStr)
                {
                    throw new Exception("Test failed");
                }
                else
                {
                    await jsRuntime.InvokeVoidAsync("alert", "Test passed");
                }
            }
        }
        catch (Exception ex)
        {
            await jsRuntime.InvokeVoidAsync("alert", ex.Message);
            logger.LogError(ex, "Test failed");
        }
    }
psxbox commented 3 days ago

I changed the C# code and the problem was solved

private async Task Test((string fileName, (string name, string desription, string type, string version)) item)
    {
        try
        {
            using (GIL.Acquire())
            {
                var fileName = Path.GetFileNameWithoutExtension(item.fileName);
                using PyObject module = Import.ImportModule(fileName);
                using var infoFunc = module.GetAttr("test");
                var testStr = "Hello, World!";
                using var pyStr = PyObject.From(testStr);
                using var result = infoFunc.Call(pyStr);
                var info = result.As<string>();
                if (info != testStr)
                {
                    throw new Exception("Test failed");
                }
            }
        }
        catch (Exception ex)
        {
            await jsRuntime.InvokeVoidAsync("alert", ex.Message);
            logger.LogError(ex, "Test failed");
            return;
        }

        await jsRuntime.InvokeVoidAsync("alert", "Test passed");
    }
atifaziz commented 3 days ago

I think your problem was the await call while holding on to the GIL. The PyGILState_Release documentation says that the GIL must be released on the same thread it was acquired on:

Every call to PyGILState_Ensure() must be matched by a call to PyGILState_Release() on the same thread.

However, the await call can potentially complete or wake up on a different thread than when it started awaiting. This is why moving the await outside the using fixes the problem. That said, two things:

  1. It's generally not a good practice to hold a lock across an await boundary.
  2. I don't think your user code needs to worry about the GIL since CSnakes should acquire and release as necessary.

On point 2, I think the exception here is Import.ImportModule, which doesn't acquire & release the GIL internally:

https://github.com/tonybaloney/CSnakes/blob/586bb13e172d82316875eb46e5eb8c5c471f470b/src/CSnakes.Runtime/Python/Import.cs#L7-L10

and neither does CPythonAPI.Import that is delegates to:

https://github.com/tonybaloney/CSnakes/blob/048e011b5d70849edb1d0c26fa9b985f8a48f6dc/src/CSnakes.Runtime/CPython/Import.cs#L13-L19

@tonybaloney can probably comment on whether Import.ImportModule was designed for public consumption or that this is an oversight.

psxbox commented 2 days ago

Thank you! I understand what it is. But unfortunately, I wanted to use it in a multitasking app.

atifaziz commented 9 hours ago

I wanted to use it in a multitasking app.

Right, but do you need the GIL for that? Couldn't you use a standard lock or synchronisation primitive to coordinate the work between the threads in your application?