Add "A Note on Concurrency"

[karagenc]

Context is not concurrent-safe. Although it is a rare condition to access the Context concurrently, I pedantically thought it should be documented.

[aldas]

I think these exaples are not necessary. I think proper explanation would be enough.

Another reason why Context is not co-routine safe is that it is borrowed from pool and release to the pool at the end of the request. But when you pass Context to another co-routine and that coroutine lifetime is longer than request own coroutine you will have situation where http.Server starts to serve another Request and that new Request gets our "leaked" echo.Context from pool. Now there are 2 completely different Requests being served with same echo.Context instance.

There are other closed issues where I have warned people not to pass echo.Context out of the Request own coroutine.

• https://github.com/labstack/echox/issues/219
• https://github.com/labstack/echo/issues/1908 (multiple comments there)
• https://github.com/labstack/echo/pull/1928#issuecomment-883567179

[karagenc]

I didn't know about the pool. I wrote an experimental package, cc4echo. It's sad to know that cc4echo doesn't solve the problem.

I don't think the examples are not necessary. It articulates the problem in a concrete way. I believe this issue should be documented.

[karagenc]

I removed examples and shortened the content. What do you think?

[aldas]

I did not want code because this will be wall of text. It would be better just to explain with couple of senteces that echo.Context fields (including standard library Request and Response instances) are not guarded by mutexes so accessing/mutating them from multiple co-routine is dangerous. Also coroutine lifetime matters as echo.Context is borrowed from pool you might up in situation where you are writing to echo.Context that is serving next Request thus writing response to different client.

I could be underestimating people but sentences like that are more easily processed/accessible than block of code. Especially when it comes to concurrency - when we struggle to write safe code - how can we expect most of the people to read and understand what we are warning them about?

And yes - I am setting bar low. This is because I think about idiots like me - how I understand/learn difficult/foreign concepts.

[karagenc]

I agree with you, so I removed the example code. Please have a look at my last commit.

Just FYI:

• Understanding foreign concepts is hard. You're not an idiot, and please don't blame yourself.
• A writing mistake: coroutine -> goroutine. Go has no coroutines,

[aldas]

@tomruk hey, sorry for late reply but could you improve this channel handling part to take into consideration that c.Request().Context() could be cancelled. Either use select or add comment to receiving line that context could be cancelled/ended before this goroutine sends its data.

Other than that - this looks good.

[aldas]

maybe comment is better. I think we should stress this out that concurrency is hard - there are many things to consider. and unless you nee to do MULTIPLE things concurrently you should not create your own goroutines in handlers - your request (connection) is already running as separate goroutine by standard library http.Server design.

[karagenc]

I updated the documentation. I didn't include the part "unless you nee to do MULTIPLE things concurrently you should not create your own goroutines in handlers - your request (connection) is already running as separate goroutine by standard library http.Server design." because I believe that would be redundant, but I added the notice "Concurrency is complicated. Beware of this pitfall when working with goroutines."