Fix internal policy of event.Bus events publishing

[voievodin]

Hi there ✋

This pull request fixed the behaviour of events publishing in Golang based event bus.
The current version of Bus.Pub func:

func (bus *Bus) Pub(e E) {
	bus.RLock()
	defer bus.RUnlock()
	cons, ok := bus.consumers[e.Type()]
	if ok {
		for idx, v := range cons {
			v.Accept(e)
			if tmpCons, ok := v.(TmpConsumer); ok && tmpCons.IsDone() {
				bus.rm(e.Type(), idx) // <= oops
			}
		}
	}
}

The main problem here is modification of bus state within the read lock in case consumer is instance of TmpConsumer and call to isDone returns true. One of the commits introduces a test-case that breaks the behaviour of this function and ends up throwing one of the following errors:

panic: runtime error: slice bounds out of range
fatal error: concurrent map writes

To solve the problem copy-on-write approach was used. So basically each time there is a new subscription or consumer removal the original array of consumers is replaced with a new one. It allows concurrently publishing go-routines to complete without any errors. This approach is similar to the one used by EventService.

[codenvy-ci]

Can one of the admins verify this patch?

[codenvy-ci]

Can one of the admins verify this patch?

[skabashnyuk]

ci-test

[garagatyi]

Hi @voievodin! Nice to see you again. Can you sign your commits to satisfy eclipse ip check?

[codenvy-ci]

ci-test build report:
Build details
Test report
selenium tests report data
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

[garagatyi]

Since the whole function is covered by a lock do we really need to copy consumers before appending the new one?

[garagatyi]

Since the whole function is covered by a lock do we really need to copy consumers before appending the new one?

[voievodin]

Technically it will work without copying, as a new element will be added to the underlining array while concurrently reading(publishing) go routines will be working with slices referencing the same array but different elements.

The reason i changed this code was to achieve the same copy-on-write pattern concept throughout all bus state modification functions. For example Bus.rm copies values because otherwise it would change the original values of underlining array, including those being concurrently referenced by reading routines, which may produce unexpected results. Fail-safe traversing, simplicity are the main reasons.

If we take a look at EventService java implementation it carries ConcurrentMapof CopyOnWriteArraySets, so each write produces a new instance of underlining array, which is the same approach in terms of modification.

[garagatyi]

I thought that it always creates a new array, but now I see that there are cases when the slice is just resliced.
Thank you for the explanation.

[codenvy-ci]

Can one of the admins verify this patch?

[voievodin]

I'm not sure i can pass CLA check as i have no access to my previous email.
I would appreciate if some of you guys commit these changes and merge them.
Thank you in advance!

[benoitf]

@voievodin just register your new gmail through Eclipse ECA

here is the link https://accounts.eclipse.org/user/login?destination=user/eca

[voievodin]

@benoitf How does it work for the users not from the Eclipse organization?
I mean if someone wants to make contribution to the project, does it mean he/she must also pass the check?

[benoitf]

@voievodin yes it's for all external contributors
All external contributors should sign ECA and add their email as Signed-Off in the commit

[voievodin]

@benoitf thank you, i will register and pass the validation =)

[voievodin]

@garagatyi, @akorneta all checks passed, feel free to merge when necessary

[benoitf]

ci-build

[skabashnyuk]

@voievodin thx