chore(cbindings): Thread-safe libwaku. WakuNode instance created directly from the Waku Thread

[Ivansete-status]

Description

This is aimed at having a safer c-bindings libwaku. With that regard, given that the WakuNode is a GC-managed type, we need to make sure the instance of the WakuNode is directly created from within the Waku Thread itself.

note: each ref, seq[T], or string are GC-manged types

Changes

• library/waku_thread/inter_thread_communication/node_lifecycle_request.nim -> New CREATE_NODE operation type.
• library/libwaku.nim -> Splitting the Waku Node creation in two parts: 1) create the Waku Thread. 2) send a CREATE_NODE req to the Waku Thread so that it creates the Waku Node instance.
• examples/cbindings/waku_example.c -> fix so that make cwaku_example actually compiles.

Issue

#1878

[vpavlin]

Should this be really hardcoded? Can you at least make it a constant?

[vpavlin]

What does this notation (the []) mean/do?

[Ivansete-status]

Thanks for the comments! I'll fix them asap.
This [] is used for ptr types. I understand the [] operator as a way to dereference the ptr, i.e. a way to access the value or object it points to.

Something interesting I've learned recently:

ref obj A	ptr A
Points to data stored in the heap	Points to data stored either in the heap or stack
GC'ed	The developer should take care of allocating and deallocating manually
No thread-safe	Thread-safe

[arnetheduck]

fwiw, [] is also used to dereference ref - in this sense, ref and ptr is similar - they differ in that ref is a reference-counted pointer and ptr is a "manual" pointer - for example:

proc myFunction(v: int) = echo v



var a = new int # this returns a `ref int`
let b = addr a[] # this returns a `ptr int` pointing to the same memory location as `a`

myFunction(a[]) # call the function with a dereferenced value
myFunction(b[])

a = nil # this removes the reference that `a` was holding - `b` now points to a memory location that the GC will reclaim - it's a dangling pointer

nim "sometimes" dereferences ref automatically as part of the language semantics - I find this to mostly be a misfeature that leads to more problems than it solves -> I always put [] when using either of ptr and ref (to remind myself that both could be nil for example)

[vpavlin]

Thank you very much!

[arnetheduck]

Suggested change

	let createThRes = waku_thread.createWakuThread()
	if createThRes.isErr():
	let msg = "Error in createWakuThread: " & $createThRes.error
	onErrCb(unsafeAddr msg[0], cast[csize_t](len(msg)))
	return RET_ERR
	let createTh = waku_thread.createWakuThread().valueOr:
	let msg = "Error in createWakuThread: " & $error
	onErrCb(unsafeAddr msg[0], cast[csize_t](len(msg)))
	return RET_ERR

[arnetheduck]

or.. if this is Result[void, E], use isOkOr:

[Ivansete-status]

Super interesting! Thanks for that! Notice that I will only apply that to this snippet and I'm planning to extend this approach to further places at nwaku.

[arnetheduck]

{.async.} functions must never call waitFor - instead, they should use await

[Ivansete-status]

ah yes, my bad!

[arnetheduck]

WakuNode is a ref object - the previous code without the ptr was correct - a consequence is that process is not a "thread-safe" function - it can only be called from the thread that created the WakuNode instance (ie the processing loop below)

[arnetheduck]

ah, I see now - this looks as if you wanted to return the node instance and async prevents var from being used - ok, ptr can be used as a workaround for that limitation as long as you keep in mind that a ptr might become dangling - this is a bit of a general problem here and for this PR it's an ok tradeoff to improve the status quo - later, I would probably look for solutions that avoid this quirk

[Ivansete-status]

Well, the idea of using ptr was to alter the node instance:

nwaku/library/waku_thread/waku_thread.nim

Line 48 in f29105b

var node: WakuNode

This instance will live enough for all the calls to any process proc and will always be owned by the Waku Thread.

[vpavlin]

lgtm

[Ivansete-status]

I will merge this as we have another PR that is blocked by that one.
Thank you so much indeed for the comments guys!
@arnetheduck - Although the PR will get merged, I can revisit any further suggestions that may arise. Thank you very much for the comments so far. I'll set you as a reviewer for the upcoming PR, where we avoid the use of the Channel type.