Skip to content
This repository has been archived by the owner on Nov 6, 2018. It is now read-only.

Rename events on PhysicalFileWatcher can throw - Crashing VS. #187

Closed
NTaylorMullen opened this issue May 9, 2016 · 11 comments
Closed

Rename events on PhysicalFileWatcher can throw - Crashing VS. #187

NTaylorMullen opened this issue May 9, 2016 · 11 comments
Assignees
Milestone

Comments

@NTaylorMullen
Copy link
Contributor

NTaylorMullen commented May 9, 2016

In some of Visual Studios test cases when they open a file => new web app 50+ times they eventually encounter crashes here. Essentially Directory.EnumerateFileSystemEntries explodes due to a node_modules file being renamed then locked.

This is a problem for Visual Studio because it ends up crashing it; however, it is infrequent. This can be mitigated by changing Mvc/Razor to only monitor the Views folder, it currently gets file watcher change events for all of the node_modules folder additions/renames during restore. This can also be fixed across the board by catching around this logic.

Not sure of the severity of this issue.

/cc @Eilon @ToddGrun @pranavkm

@NTaylorMullen NTaylorMullen changed the title Rename events on PhysicalFileWatcher can throw. Rename events on PhysicalFileWatcher can throw - Crashing VS. May 9, 2016
@Eilon
Copy link
Member

Eilon commented May 9, 2016

@NTaylorMullen this shouldn't crash VS no matter what. Did you open a VS bug?

@ToddGrun
Copy link

ToddGrun commented May 9, 2016

VS doesn't own any of the code on this stack. The exception never gets caught and ends up crashing VS.

@Eilon
Copy link
Member

Eilon commented May 9, 2016

@ToddGrun hmm then how does it crash VS?

@ToddGrun
Copy link

ToddGrun commented May 9, 2016

@Eilon -- An unhandled exception on a background thread will kill VS.

@Eilon
Copy link
Member

Eilon commented May 9, 2016

Ah, background thread, got it!

@Eilon
Copy link
Member

Eilon commented May 9, 2016

@muratg @DamianEdwards should we consider a hot-fix here for RC2 to at least try-catch? And do a better fix for RTM?

@muratg
Copy link

muratg commented May 9, 2016

@Eilon yes, I think a simple fix in RC2 is worthy, considering how bad the experience is.

@Eilon
Copy link
Member

Eilon commented May 10, 2016

@muratg can you send email for approval?

@muratg muratg added the bug label May 10, 2016
@muratg muratg added this to the 1.0.0-rc2 milestone May 10, 2016
NTaylorMullen added a commit that referenced this issue May 10, 2016
- When enumerating files in the `e.FullPath` directory it's possible to get IO exceptions due to renames, folder structure changes and file locks. This swallows those exceptions.

#187
NTaylorMullen added a commit that referenced this issue May 10, 2016
- When enumerating files in the `e.FullPath` directory it's possible to get IO exceptions due to renames, folder structure changes and file locks. This swallows those exceptions.

#187
@NTaylorMullen
Copy link
Contributor Author

NTaylorMullen commented May 10, 2016

98ea34e

@DamianEdwards
Copy link
Member

@NTaylorMullen that commit is in to dev, is this going in to release as well?

@NTaylorMullen
Copy link
Contributor Author

Oops pasted the dev commit instead of the release commit (it went into release first).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants