{"action":"created","issue":{"url":"https://api.github.com/repos/open62541/open62541/issues/947","repository_url":"https://api.github.com/repos/open62541/open62541","labels_url":"https://api.github.com/repos/open62541/open62541/issues/947/labels{/name}","comments_url":"https://api.github.com/repos/open62541/open62541/issues/947/comments","events_url":"https://api.github.com/repos/open62541/open62541/issues/947/events","html_url":"https://github.com/open62541/open62541/issues/947","id":202833805,"number":947,"title":"Add-Node in multithreading mode can lead to BadOutOfMemory-Error","user":{"login":"janitza-luwe","id":24267637,"avatar_url":"https://avatars.githubusercontent.com/u/24267637?v=3","gravatar_id":"","url":"https://api.github.com/users/janitza-luwe","html_url":"https://github.com/janitza-luwe","followers_url":"https://api.github.com/users/janitza-luwe/followers","following_url":"https://api.github.com/users/janitza-luwe/following{/other_user}","gists_url":"https://api.github.com/users/janitza-luwe/gists{/gist_id}","starred_url":"https://api.github.com/users/janitza-luwe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/janitza-luwe/subscriptions","organizations_url":"https://api.github.com/users/janitza-luwe/orgs","repos_url":"https://api.github.com/users/janitza-luwe/repos","events_url":"https://api.github.com/users/janitza-luwe/events{/privacy}","received_events_url":"https://api.github.com/users/janitza-luwe/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-01-24T14:30:49Z","updated_at":"2017-01-26T10:00:13Z","closed_at":null,"body":"\r\n[fix_UA_Server_addDataSourceVariableNode.patch.txt](https://github.com/open62541/open62541/files/729639/fix_UA_Server_addDataSourceVariableNode.patch.txt)\r\n\r\nDescription\r\n===========\r\nThere is a Problem in multithread server when adding Nodes with _UA_Server_addDataSourceVariableNode()_. When doing this on a slower machine (ARM9 450Mhz in my case) with high scheduling rate, that function sometimes returns a \"UA_STATUSCODE_BADOUTOFMEMORY\"-Error. \r\n\r\n\r\nBackground Information / Reproduction Steps\r\n===========================================\r\nExample code is unnecessary, imagine any code that creates a new Node with _UA_Server_addDataSourceVariableNode()_.\r\n\r\nFirst this method creates a new node in Nodestore with _UA_NodeStore_newVariableNode()_ . After that the Node gets set up and subsequent methods are called with the **references** to this Node/Nodeid as parameter argument. One possible callstack is:\r\n\r\n_UA_Server_addDataSourceVariableNode()_\r\n--> _Service_AddNodes_existing()_\r\n--> _instantiateNode()_\r\n--> _Service_AddReferences_single()_\r\n--> _UA_Server_editNode()_\r\n\r\nThis call to _UA_Server_editNode()_ in multithreading mode leads to a replacement of the Node in Nodestore by _UA_NodeStore_replace()_ --> **call_rcu(&oldEntry->rcu_head, deleteEntry)**\r\n\r\nThe deletion of the old Node in Nodestore is done by liburcu in a separate Thread (strace showed a syscall to 'clone()' which leads me to the problem). Since deletion is done without any locks, this happens as soon as the scheduler switches to the new created liburcu thread.\r\n**The problem is, that the node-creating process continues working with the reference to the original created Node/NodeId (which is possibly deleted concurrently)!** \r\nIn further execution another call to _UA_Server_editNode()_ will lead to a call to _UA_NodeStore_getCopy()_ with a NodeId-Reference to the (possibly) deleted Node, which is all 0 in that case. The lookup will fail to find the NodeId and  _UA_NodeStore_getCopy()_ returns NULL (which results in a UA_STATUSCODE_BADOUTOFMEMORY)\r\n\r\nA solution could be to add a new Function _deleteEntry_rcu() that takes RCU-Lock before deleting the Node. This will preserve the referenced NodeID until RCU lock is released in _UA_Server_addDataSourceVariableNode()_.\r\n\r\nEDIT: Locking deleteEntry seems not to be sufficient, i think i misunderstood locking at all (rcu_read_unlock() is never called if NDEBUG is not set?)\r\nMy solution is to create a local copy of node->nodeId in Service_AddNodes_existing() after UA_NodeStore_insert() and work with that local copy from there on.\r\n\r\nChecklist\r\n=========\r\n - [ ] open62541 Version (release number or git tag): Branch 0.2,  Commit-Id 21b8c18\r\n - [ ] Operating system: Linux 4.6 on ARM9 (ARMv5te)\r\n - [ ] Patch for master branch\r\n"},"comment":{"url":"https://api.github.com/repos/open62541/open62541/issues/comments/275350073","html_url":"https://github.com/open62541/open62541/issues/947#issuecomment-275350073","issue_url":"https://api.github.com/repos/open62541/open62541/issues/947","id":275350073,"user":{"login":"jpfr","id":1726411,"avatar_url":"https://avatars.githubusercontent.com/u/1726411?v=3","gravatar_id":"","url":"https://api.github.com/users/jpfr","html_url":"https://github.com/jpfr","followers_url":"https://api.github.com/users/jpfr/followers","following_url":"https://api.github.com/users/jpfr/following{/other_user}","gists_url":"https://api.github.com/users/jpfr/gists{/gist_id}","starred_url":"https://api.github.com/users/jpfr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jpfr/subscriptions","organizations_url":"https://api.github.com/users/jpfr/orgs","repos_url":"https://api.github.com/users/jpfr/repos","events_url":"https://api.github.com/users/jpfr/events{/privacy}","received_events_url":"https://api.github.com/users/jpfr/received_events","type":"User","site_admin":false},"created_at":"2017-01-26T10:00:13Z","updated_at":"2017-01-26T10:00:13Z","body":"Hello @janitza-luwe,\r\n\r\n> The problem is, that the node-creating process continues working with the reference to the original created Node/NodeId (which is possibly deleted concurrently)!\r\n\r\nThat should not happen. Deleting nodes is delayed until all threads leave their current read-side ciritical section. I'll have a look at this. But I don't have time to deep-dive into the issue right now."}}
{"actor":{"display_login":"jpfr"}}