Skip to content

Conversation

kantora
Copy link
Contributor

@kantora kantora commented Jul 29, 2016

Sorry, I've replaced one bug with another. This should help.

@kantora kantora force-pushed the DistributedPubSubMediatorHandlePruneFixFix branch from 6e31eb5 to 3b7c854 Compare July 29, 2016 11:44
@kantora
Copy link
Contributor Author

kantora commented Jul 29, 2016

Hmmm... There is no more exception here, but it still works wrong... Now it silently drops active subscriptions.

@kantora kantora changed the title Fix of fix in #2222 (removed multiple enumeration) DistributedPubSubMediator - an error in HandlePrune, Fix of fix in #2222 (removed multiple enumeration) Jul 29, 2016
@kantora
Copy link
Contributor Author

kantora commented Jul 29, 2016

The original pull request: #2222
The original issue: #2221

@alexvaluyskiy
Copy link
Contributor

alexvaluyskiy commented Aug 2, 2016

@kantora I can't merge this PR, because we don't have any test for "HandlePrune" neither in MNTK tests nor in unit tests. Could you create the one? If not, just share some examples of code, which can help me to reproduce this bug

@kantora
Copy link
Contributor Author

kantora commented Aug 2, 2016

@alexvaluyskiy As for me, it reproduces just each time cluster pub/sub is used, if you give it at least 2 or 3 minutes to run. And, to be honest, this pr just hides the problem (as it prevents the exception to appear) and don't fixes it. I've described the main issue in #2226.

As for this particular PR - the fix is obvious from code as the previous version modified dictionary while was iterating it and fixes the recently merged PR #2222.

@alexvaluyskiy
Copy link
Contributor

@kantora I've tested your code, it really fixed the problem. But we shouldn't test our code manually. So we should create a test for it. I've created an issue for this case
#2232

@alexvaluyskiy alexvaluyskiy merged commit 3b820cc into akkadotnet:dev Aug 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants