[Fix] MultiSyncModeSelector.Changed event propagation being blocked by SyncFeed

[smartprogrammer93]

Fixes #6481

Changes

• SyncFeed's Intialize method takes a while. Call to it made async.
• Log messages are only printed out once SyncFeed finishes initialization.

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

[benaadams]

Interesting solution! 😊

[LukaszRozmej]

Why this is solving the issue? What is the technical cause of the issue? If someone is blocking the Changed handler, maybe that subscription needs to be parallelized and not the invocation?

[LukaszRozmej]

I think it should not happen, but in theory pattern is to do a local copy to avoid NullReferenceException and be thread-safe:

https://stackoverflow.com/questions/33166143/local-copy-of-event

[smartprogrammer93]

😲 You learn something every day

[asdacap]

What was the problem?

[benaadams]

What was the problem?

Multiple events subscribed to the event; important one is second, first one takes a while to return and they are executed in order

[smartprogrammer93]

@LukaszRozmej not entirely sure which delegate is blocking the invocation. also, parallelizing the the delegate method could lead to its own problems in my opinion (with concurrency and race conditions). Additionally, This solution guarantees that all delegates finish executing before continuing to execute the rest of the code.

[smartprogrammer93]

I will not merge this till it is properly tested and be sure it will resolve the issue. @kamilchodola waiting on your confirmation, as i am not sure how exactly you were able to produce it. was not able to do so.

[LukaszRozmej]

@LukaszRozmej not entirely sure which delegate is blocking the invocation. also, parallelizing the the delegate method could lead to its own problems in my opinion (with concurrency and race conditions). Additionally, This solution guarantees that all delegates finish executing before continuing to execute the rest of the code.

Well it makes it explicit and something to handle by the end-user, which actually allows for proper handling of race conditions. Now it is hidden.

[flcl42]

Can we make so sync starts only when migrations are done?

[LukaszRozmej]

What was the problem?

Multiple events subscribed to the event; important one is second, first one takes a while to return and they are executed in order

I would execute first one parallel explicitly in the handler, don't you think it is better?

[smartprogrammer93]

@LukaszRozmej i disagree. since if another syncmode change happens while the handler is still executing the previous one it could lead to a race condition.
With this approach, the parallel for guarantees the execution of all of the handlers is done before it releases the thread possibly blocking syncmode from switching before all the previous handlers have already finished execution.

[kamilchodola]

@smartprogrammer93 @LukaszRozmej what about this one?

[smartprogrammer93]

Ready from my side. Feel free to merge it if no more testing is needed. @kamilchodola