Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disposing the FileSystemWatcher does not close associated file descriptors #16709

Closed
mrward opened this issue Sep 6, 2019 · 5 comments
Closed

Disposing the FileSystemWatcher does not close associated file descriptors #16709

mrward opened this issue Sep 6, 2019 · 5 comments
Assignees

Comments

@mrward
Copy link
Member

@mrward mrward commented Sep 6, 2019

Steps to Reproduce

Related problem - #15931

  1. Sample project - https://github.com/mrward/test-file-watcher-dispose - multi-target project that builds .NET 4.7.2 and .NET Core 2.1
  2. Build this project.
  3. From the command line run mono bin/debug/net472/FileWatcherDisposeTest.exe
  4. Run lsof -p pid. Process id is output from the console app.
  5. See that the file watcher creates file descriptors for several directories.
  6. After 10 seconds the file watcher will be disposed.
  7. Run lsof -p pid again.

Current Behavior

lsof still lists the directories.

mono-sgen 3972 user    4r DIR   1,12      170   51740695 /Volumes/Drive/projects/tests/FileWatcherDisposeTest/FileWatcherDisposeTest/bin/Debug/net472
mono-sgen 3972 user    5r DIR   1,12      136   51740600 /Volumes/Drive/projects/tests/FileWatcherDisposeTest/FileWatcherDisposeTest/bin/Debug
mono-sgen 3972 user    6r DIR   1,12      102   51740599 /Volumes/Drive/projects/tests/FileWatcherDisposeTest/FileWatcherDisposeTest/bin
mono-sgen 3972 user    7r DIR   1,12      204   51740594 /Volumes/Drive/projects/tests/FileWatcherDisposeTest/FileWatcherDisposeTest
mono-sgen 3972 user    8r DIR   1,12      170   51740593 /Volumes/Drive/projects/tests/FileWatcherDisposeTest
mono-sgen 3972 user    9r DIR   1,12     6324     185300 /Volumes/Drive/projects/tests
mono-sgen 3972 user   10r DIR   1,12     2312        278 /Volumes/Drive/projects
mono-sgen 3972 user   11r DIR   1,12      646          2 /Volumes/Drive
mono-sgen 3972 user   12r DIR    1,4      160     420284 /Volumes

If you change the console project to run GC.Collect after the dispose then lsof will not list the directories.

Also affects .NET Core 2.1 (bug the GC.Collect does not workaround the problem there)

Expected Behavior

File watcher directories not listed by lsof.

Note that

On which platforms did you notice this

[x] macOS
[ ] Linux
[ ] Windows

Version Used:

Mono JIT compiler version 6.4.0.188 (2019-06/2c3aeaf3780 Tue Sep 3 15:10:54 EDT 2019)

Stacktrace

@steveisok

This comment has been minimized.

Copy link
Contributor

@steveisok steveisok commented Sep 6, 2019

@mrward Did you create an upstream corefx issue as well?

@mrward

This comment has been minimized.

Copy link
Member Author

@mrward mrward commented Sep 6, 2019

No, I have not done that. Was investigating Mono since this affects VS Mac. I can open one against dotnet/corefx and reference this.

@mrward

This comment has been minimized.

Copy link
Member Author

@mrward mrward commented Sep 6, 2019

Opened this also against corefx - dotnet/corefx#40888

lambdageek added a commit to lambdageek/mono that referenced this issue Sep 18, 2019
The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.

Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance.  (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).

However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling

    watcher?.StopDispatching (watcher_handle);
    watcher?.Dispose (watcher_handle);

which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.

Addresses mono#16709
monojenkins added a commit to monojenkins/mono that referenced this issue Sep 18, 2019
The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.

Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance.  (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).

However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling

    watcher?.StopDispatching (watcher_handle);
    watcher?.Dispose (watcher_handle);

which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.

Addresses mono#16709
monojenkins added a commit to monojenkins/mono that referenced this issue Sep 18, 2019
The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.

Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance.  (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).

However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling

    watcher?.StopDispatching (watcher_handle);
    watcher?.Dispose (watcher_handle);

which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.

Addresses mono#16709
monojenkins added a commit to monojenkins/mono that referenced this issue Sep 18, 2019
The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.

Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance.  (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).

However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling

    watcher?.StopDispatching (watcher_handle);
    watcher?.Dispose (watcher_handle);

which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.

Addresses mono#16709
monojenkins added a commit that referenced this issue Sep 19, 2019
[2019-08] [System] Make FileSystemWatcher backend non-static

The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.

Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance.  (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).

However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling

    watcher?.StopDispatching (watcher_handle);
    watcher?.Dispose (watcher_handle);

which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.

Addresses #16709



<!--
Thank you for your Pull Request!

If you are new to contributing to Mono, please try to do your best at conforming to our coding guidelines http://www.mono-project.com/community/contributing/coding-guidelines/ but don't worry if you get something wrong. One of the project members will help you to get things landed.

Does your pull request fix any of the existing issues? Please use the following format: Fixes #issue-number
-->


Backport of #16922.

/cc @steveisok @lambdageek
steveisok added a commit that referenced this issue Sep 20, 2019
* [System] Make FileSystemWatcher backend non-static

The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.

Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance.  (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).

However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling

    watcher?.StopDispatching (watcher_handle);
    watcher?.Dispose (watcher_handle);

which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.

Addresses #16709
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Sep 20, 2019
Changes: mono/mono@7ec17ba...bfbf823

Context: mono/mono#16709
Context: xamarin/xamarin-macios#7005

Disables `ppdb` data decompression.
Add missing membars when initializing rgctx entries
Remove the mac32 build
Adds .NET 4.8 reference assemblies
@lambdageek

This comment has been minimized.

Copy link
Member

@lambdageek lambdageek commented Sep 20, 2019

This is fixed on master and on 2019-08 now.

@lambdageek lambdageek closed this Sep 20, 2019
@Therzok

This comment has been minimized.

Copy link
Member

@Therzok Therzok commented Sep 22, 2019

@slluis do we want this in 8.3 too?

jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 24, 2019
Changes: mono/mono@7ec17ba...bfbf823

Context: mono/mono#16709
Context: xamarin/xamarin-macios#7005

Disables `ppdb` data decompression.
Add missing membars when initializing rgctx entries
Remove the mac32 build
Adds .NET 4.8 reference assemblies
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 24, 2019
Changes: mono/mono@7ec17ba...bfbf823

Context: mono/mono#16709
Context: xamarin/xamarin-macios#7005

Disables `ppdb` data decompression.
Add missing membars when initializing rgctx entries
Remove the mac32 build
Adds .NET 4.8 reference assemblies
@Therzok Therzok added this to Needs triage in VS4M Tracker via automation Dec 23, 2019
@Therzok Therzok moved this from Needs triage to Closed in VS4M Tracker Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
VS4M Tracker
  
Closed
5 participants
You can’t perform that action at this time.