Skip to content

Conversation

@smoothdeveloper
Copy link
Collaborator

based on investigation from @mrinaldi in #241 and suggestion from @dsevastianov to clear the cache instead of disposing it.

I confirm that in my case it doesn't crash anymore.

…ojects#241 and suggestion from @dsevastianov to clear the cache instead of disposing it.
@dsevastianov
Copy link
Contributor

@smoothdeveloper This looks much better. The only suggestion is to do actual dispose in this.Disposing.Add, here we do want to get rid of the cache.

@smoothdeveloper
Copy link
Collaborator Author

@dsevastianov good catch, I've just added that explicit call.

@dsevastianov
Copy link
Contributor

Awesome, thanks! One last thing, sorry I didn't catch this earlier - this disposingCache variable should be isDisposing and should be only checked in this.Disposing handler. Here's an explanation, we clear cache on each monitor implementation but clear cache and notification on each monitor change. I hope it makes sense, sorry for all the iterations :)

…st it)

* renamed clearingCache to isDisposing and using it only in IDisposable.Dispose implementation
@smoothdeveloper
Copy link
Collaborator Author

@dsevastianov thanks for feedback, I'm not sure I'm following, I renamed the variable clearingCache to isDisposing and only flip it in Dispose implementation, and I also extracted a class for the cache because I think we eventually want to put it under tests.

I hope it looks ok now :)

@dsevastianov dsevastianov merged commit 077e30c into fsprojects:master Dec 29, 2016
@dsevastianov
Copy link
Contributor

@smoothdeveloper Released 1.8.3-beta4, thanks for your patience!

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.

2 participants