Improvement: Too much defer usage

[OmidHekayati]

Description

defer must use just when we are not sure a bad thing (caused by used panic() in other places!) may occur in a process(goroutine).

How to solve

easily find and replace all defer with simple flow! e.g. Fix state.state.Genesis() to this version:

func (st *state) Genesis() (gg *genesis.Genesis) {
	st.lk.RLock()
        gg = st.genDoc
	st.lk.RUnlock()
	return
}

EDIT: Read deeply about defer mechanism here
defer and reflection calls can build frames in the heap with the appropriate heap bitmap. The call bridge in these cases must open a new stack frame, copy stack to the stack, load the register arguments, call pc, and then copy the register results and the stack results back to the in-heap frame (using write barriers where necessary). It may be valuable to have optimized versions of this bridge for tail-calls (always the case for defer) and register-only calls (likely a common case). In the register-only reflection call case, the bridge could take the register arguments as arguments itself and return register results as results; this would avoid any copying or write barriers.

[b00f]

We usually use defer for locks, and it is a common practice in all code: the first two lines of the function is for locking.
In the provided example, we're breaking this common practice. Plus, a return value name has been defined (which I personally don't like it).

I understand that encapsulating a shared and guarded variable within the lock, could result in a shorter lock time, but it comes with some costs.
Let's consider an example:

func (g *guardedClass) DoSomething() {
    g.lk.Lock()
    defer g.lk.Unlock()

    g.foo()

    g.noNeedGuard()

    g.bar()
}

noNeedGuard is a sub-function that doesn't require guarding, and a lock mechanism is unnecessary. We could re-write the code as follows:

func (g *guardedClass) DoSomething() {
    g.lk.RLock()
    g.foo()
    g.lk.RUnlock()

    g.noNeedGuard()

    g.lk.RLock()
    g.bar()
    g.lk.RUnlock()
}

There are a few considerations about the second example:

1. Locking and unlocking are costly operations. In the first example, we have one lock, but in the second, we have two.
2. We need to be very cautious to ensure that each lock gets unlocked in the appropriate place, especially within loops.
3. Distributing locks within the function body can make it harder to read.

If the noNeedGuard function isn't a heavy task and its runtime is negligible, we can include it within the main lock (example 1)

To sum up: the question is, how much should be the scope of a lock? Should it be the entire function scope or limited to the guarded variables?

It seems that for most cases the first approach (example 1) is a good choice.
However, we can do some benchmark tests to make a better decision.

[kehiy]

about context, I think we need a refactor
and it has 3 to 4 steps.
first of all, we need to use context.TODO() for tests
then check the best practice of where we should define and keep them (in structs or ...)
then refactor the code with the best practice and after it just makes a global context and passes it to all modules.

[b00f]

@kehiy you have god points here. As you said we need to refactor. We know what is wrong here.

For tests we can add a helper function in test suite. any idea? How to do that?
I think the best place is inside the test suite.

[kehiy]

about the context in the test, I just know we need to use the TODO context from @ja7ad 's help and some research.
now I think we can make a context.TODO() in the test suite and use it everywhere, but are you sure we are using the test suite for all of the tests?

[b00f]

I prefer to not merge your PR until we really really make sure it is the best way.
We shouldn't to change this part again.

@OmidHekayati

Could you please review this PR? #582

[kehiy]

i'm agree with this I also mentioned this in our meeting this day!
we should close it, start research and refactor the contexts in the code in the correct way.
because the context issues are more than just making it globally in the node.