Implement coroutines to make value-callbacks async aware

[swamper123]

make value_callback coroutine able
make changes methods and tests to make this able

[oroulet]

@swamper123 be carefull to not merge but use rebase to keep a somewhat clean pull request (ususally git pull -r is enough)

[oroulet]

Sorry this took far too long from my side. Can you try to rebase and I will merge.

[swamper123]

@oroulet No worries, I had a small break as well. I'm already on it, but this may take another day. As well I'm still thinking about your concern of undefined behaviour. Do you know sth. about other OPC UA Stacks how they handle it? I storngly believe we are not the only one out there with such a problem.

[oroulet]

Undefined behavior?

[swamper123]

This PR is based on my (messy) old one, where you mentioned that race conditions may happen.

#138 (comment)

[oroulet]

Ok. I forgot that stuff.... Yes there is a potential issue here... Why do you need these methods async?

[swamper123]

My build up is a ("stupid-non OPC able") PLC and OPC UA server and I use a script to let them communicate (using asyncio). Instead of polling every xxx ms/s, I want to trigger a request process. This is where value_callback is used. If somebody wants to read the Value of a Node , the async request process starts.

With a Coroutine I wouldn't have the problem, if the value is legit or not (because the Client receives a legit answer of the request process).

I also got nodes which need a async func/method (between server and PLC) to even get a legit value. E.g. the temperature values in the PLC are in °C. Now I got some Nodes with Kelvin and Fahrenheit as well. If I want to read my Node with the °C value, I want to trigger the async value_callback, request, grab the values, calculate the stuff in Kelvin, Fahrenheit and Celsius and put the new values into their nodes as well.

So my thought was, if it would be possible to add coros into the value_callback, I wouldn't have problems with my async communication Script, I could run even time intense async processes without (greater) performance loss incl. other coros timing out and it would be a good feature beside the sync value_callbacks as well.

A problem could still be a race condition, in case a Node is deleted before an async value_callback may have finished. I don't know how asyncio & asyncua are handling cancelled Tasks in such a case.

I posted 3 possible ideas in the last PR for a workaround:

• One idea is to add an Attribute value_callback_handler to AttributeValue, where the handler of the value_callback coro could be stored.
  With that, the _delete_node method in address_space.py could be extended by a request,if a callback task exist. If yes, cancel it (maybe with a logger_warning), and continue the deletion process. Canceling a task, if the handler is available, would be sync as well.
• Another Idea would be, if a value_callbacktask is running and the Node shall be deleted, to use asyncio.add_done_callback to let the read-process finish and start the deletion process of the node directly afterwards.
• Another Idea is to add a flag "deletion_progress" and to deny further operations with the Node, until all tasks are done.

What I don't know is, how asyncua may handle with such problems in gerneal. Like if somebody add race conditioning stuff in:

opcua-asyncio/asyncua/server/address_space.py

Line 697 in c878552

def add_datachange_callback(self, nodeid, attr, callback):

or in

opcua-asyncio/asyncua/server/address_space.py

Line 716 in c878552

def add_method_callback(self, methodid, callback):

[swamper123]

Well it seems that we have a problem with the python versions. Test works fine for me on Python 3.8, but seems to crash here on 3.6.7 in particular cases.

[oroulet]

@swamper123 discuss things here, so we do not needto look at differnt places to get overview.
Also I seewe have a race condition issue. if we merge that we need to add a lock to address space again... (it got away when we moved to asyncio)

[swamper123]

@oroulet So add_method_callback and add_datachange_callback have to be removed as well?

[oroulet]

Remove? No but if we make them async we need to add an asyncio.Lock in address space so two tasks cannot modify address space at the same time

[swamper123]

I haven't noticed that they aren't async. But even if they aren't async, isn't it possible that someone throws a def delete_node into them, while another Task is awaiting for a Client response? So it would be the same behaviour,
To be more clear:

• Task1 on is awaiting a Value
• Task2 is on his turn, where a datachange happened and in sync delete_node was called which shall delete the target Node of Task1
• Task2 finishes and Task1 is on it's turn again -> race condition