fix: changed eventing configuration mutex to rwmutex and added missing lock

[skyerus]

This PR

Changed eventing configuration mutex to rwmutex and added missing lock.

Related Issues

Fixes #219

Notes

Follow-up Tasks

How to test

[james-milligan]

I believe these have a similar outcome due to the wrapping for loop. Original intention was to keep this as a timeout, as we don need to send a keep alive if either the request is closed, or we send a notification (this is to prevent read/write timeouts)

[beeme1mr]

@AlexsJones, could you please review this when you have a moment?

[AlexsJones]

I might be wrong here but

	requestNotificationChan := make(chan Notification, 1)
	s.eventingConfiguration.mu.Lock()
	s.eventingConfiguration.subs[req] = requestNotificationChan
	s.eventingConfiguration.mu.Unlock()

If you have two goroutines calling the eventStream function the second will have it's write queued.
I think what it's missing is a check prior to the lock to see if requestNotificationChan := make(chan Notification, 1) has been set at all.

[skyerus]

I'm not following the concern here, each invocation of EventStream() will lock while setting the channel to the service's state of ongoing connections.

I think what it's missing is a check prior to the lock to see if requestNotificationChan := make(chan Notification, 1) has been set at all.

Each EventStream() has its own channel, in what scenario would requestNotificationChan have failed to have been set?

[skyerus]

Is the concern that we're overwriting an already existing channel in the map?
The key is a pointer to the request structure, this is unique (at least until garbage collected).

[AlexsJones]

I can't see any tangible benefits to this so I defer to others.

[skyerus]

The benefit is that we're avoiding a potential race condition from writing to the same map concurrently.