Named parameters in VarPyth not cleared between function calls

[mce2]

Under some circumstances (sorry, I don't have minimal code to reproduce the error) named parameters in VarPyth not cleared between function calls, and function called without named parameters may recieve parameters from previous call with named parameters. Condition check in line 1091 of VarPyth.pas seems to be excessive; in my case commenting it out solved the problem:
{if CallDesc^.NamedArgCount > 0 then }GetNamedParams;

[mce2]

I found that this happens in multithread environment: if I call python function with named parameters in one thread, and this call is not finished when I call another function in another thread without named parameters, the second functions receives named parameters from the previous call. Is VarPyth thread safe, and is my way of overcoming the problem safe?

[pyscripter]

Python is not thread safe. Google for "Python GIL".
Before you try to run python code in threads, make sure you understand python threading and study Demo 33.

[mce2]

I know about GIL, and all my calls are executed between
gilstate := PyGILState_Ensure();
PyGILState_Release(gilstate);
The python code works fine if I execute it via TPythonEngine.ExecString(). The problem is in VarPyth: it reuses the same TNamedParamArray for all calls, but the former call may be in progress while the latter is started (in my case the former starts wx.App MainLoop, so it does not exit untill the application is finished).

[mce2]

Assumption that one can enter only one function using VarPyth interface, is incorrect. GIL lock may be released and re-acquired inside python code (during time.sleep, IO operations, in GUI message loop, etc.), so in multy-threaded application it's generally normal to have more than one function running inside the same python interpreter.

[pyscripter]

Can you submit a patch?

[mce2]

I'm not sure if the patch I currently use can cause other problems. I'll try to investigate the problem more thorougly and make a general silution.

[pyscripter]

I don't see how your PR resolves the issue.

The problem is that fNamedParams is stored as a field of TPythonVariantType (singleton class). If multiple threads use Python Variants to make calls to Python you will still have problems,

Possible Solutions:

• Make fNamedParams a Global threadvar, but threadvars come with their own performance penalty and limitations.
• Store fNamedParams in a threadlist with the threadid and retrieve from there.
• Store fNamedParams in TPythonData (one per variant) instead of TPythonVariantType (singleton class)
• Store named arguments in the Arguments TVarDataArray, using some marker to separate standard from named arguments.
• Protect DispInvoke with a Critical Section, so that it cannot be called by a different thread before finishing execution. This seems to be the easiest to implement option.

Why this flawed design

DispInvoke calls DoProcedure and DoFunction which then use EvalPython to do the work. The inherited DispInvoke does not make use of or deal with Named arguments and it needs to be overridden as well as DoProcedure and DoFunction. They all have a fixed signature, so you cannot for example pass another parameter from DispInvoke to DoFunction and you need to have some other means of passing fNamedParams from DispInvoke to DoFunction and then to EvalPython.

[mce2]

You are absolutely right, my code is not a resolution but a dirty workaround. However it looks like GIL will do the magic, and this code should work correct. As I understand VarPyth code, named parameters are transfered via fNamedParams to python dict object by calling PyDict_SetItemString in a loop (line 1676). If I am correct, we may SetLength(fNamedParams, 0) immidiately after this loop. I assume that due to GIL execution of the thread cannot be interrupted between setting fNamedParams and calling PyDict_SetItemString, so this should be OK. This workaround is stinky, but it should work. Sorry, I'm not experienced enough to develop a real solution for the issue.

[pyscripter]

@mce2
It took quite a while...
The solution is similar to what you originally suggested. It also makes sure that fNamedParams is cleared asap and before the python method is executed.

Could you please test it?