Bug 198727 - [PATCH] lang/mono FileSystemWatcher (kevent) deadlock problem
Summary: [PATCH] lang/mono FileSystemWatcher (kevent) deadlock problem
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-mono mailing list
URL:
Keywords: patch
: 206593 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-03-20 08:39 UTC by Ivan Radovanovic
Modified: 2019-09-15 10:18 UTC (History)
15 users (show)

See Also:
bugzilla: maintainer-feedback? (mono)


Attachments
patch - ready to go into files directory of port (2.26 KB, text/x-csharp)
2015-03-20 08:39 UTC, Ivan Radovanovic
no flags Details
patch files for mono 4.2.2.10 (12.39 KB, application/octet-stream)
2016-01-26 00:04 UTC, Ivan Radovanovic
no flags Details
patch files (12.35 KB, application/x-xz)
2016-01-26 09:14 UTC, Ivan Radovanovic
no flags Details
log of jackett crashing at startup on kevent(2) (4.89 KB, text/plain)
2019-01-24 14:28 UTC, Michiel van Baak
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Radovanovic 2015-03-20 08:39:09 UTC
Created attachment 154559 [details]
patch - ready to go into files directory of port

There were 2 problems with mono's original implementation of kevent FileSystemWatcher on FreeBSD:

* FileSystemWatcher implementation in mono's HEAD for kevent was relying on Darwin's specific behavior (when kevent is executed with NULL timeout, if kqueue FD is closed kevent call on Darwin returns with error, however FreeBSD doesn't behave that way - in fact according to feedback I got from kernel developers on FreeBSD close would block waiting for kevent to complete thus causing deadlock)

* All watcher events were generated from the same thread which detected them and that presented possibility of deadlock if change handler was to disable/reset watcher (this manifested in ASP.Net applications not being restarted after dll changes, and not recompiling individual components on aspx/ascx file changes)

Attached patch fixes those problems (I was able to verify that after applying it XSP and mono's fastcgi servers behave in expected way).

Attached patch does several things:

* renames member of imported timespec structure from misleading tv_usec to proper tv_nsec (value is in nanoseconds, not microseconds)

* adds watcher thread abort in case it doesn't gracefully shutdown within 2 seconds (I think not really needed, but better safe than sorry)

* moves actual event dispatching to separate thread (thus eliminating potential for deadlock described earlier)

Attached patch is to be applied on latest version of port at this moment (3.12.1)
Comment 1 Romain Tartière freebsd_committer 2015-05-17 17:11:09 UTC
Have this problem been reported upstream?  Can you please provide a link to the upstream issue?

Thanks!
Comment 2 Ivan Radovanovic 2015-05-18 07:39:47 UTC
No, according to my experience they are excellent at ignoring patches and bug reports for anything which doesn't concern Linux (and lately OSX), so I didn't want to waste my time. If you have better communication with them you can try to report it and lets see what happens :-)
Comment 3 Ben Woods freebsd_committer 2015-05-19 22:29:46 UTC
Hi Ivan,

Thanks for submitting this patch - I am the maintainer of emby-server for FreeBSD (which uses mono), and the file system watcher does not work, meaning whenever new files are added to your media library you must manually initiate a library scan. I am hopeful your patch will resolve this!

I have submitted 2 patches upstream to mono recently, and both have been accepted now. One was accepted very quickly, whilst the other took just over a month. I think the secret is to ensure that you patch does not adversely affect Linux or Mac OSX functionality, just to fix BSD functionality. It needs to work for both parties.

The way to submit your changes to mono upstream is to fork mono on GitHub, make your changes to your GitHub repo, and then make a pull request to the main mono project code on GitHub. I typically list the FreeBSD bug report in the pull request description.

Here are the 2 I had accepted:
https://github.com/mono/mono/pull/1390
https://github.com/mono/mono/pull/1404

Good luck, and thanks again!
Comment 4 Daniel Becker 2015-09-28 21:49:16 UTC
Has there been any progress on this? If not, could the patch be included in the port until the fix has been upstreamed?
Comment 5 Daniel Becker 2016-01-05 11:41:01 UTC
(In reply to Ben Woods from comment #3)
Ben, I believe the real problem for the FreeNAS port is that as of mono-4.2.1.124, the kqueue-based watcher has a hard-coded limit of 200 open file descriptors; guessing that most Emby libraries have way more files/directories than that. This limit appears to be arbitrary and its actually been increased to a default of Int32.MaxValue in master (tho not in 4.2.2.10).
Comment 6 Daniel Becker 2016-01-05 11:52:26 UTC
I filed PR 205919 with a patch to remove the 200 FD limit.
Comment 7 Ivan Radovanovic 2016-01-26 00:03:01 UTC
I finally found some time to implement this watcher properly for FreeBSD (note: this probably makes it incompatible with OSX (I don't have way to test)). 

File descriptor limit is completely removed and unless files themselves are monitored for modification (ie for size change, attribute change and similar) descriptors are used only for directories. Solution is as fast as possible while still being as close to Microsoft's spec as possible (unfortunately on FreeBSD that means re-reading directory after each modification (although that is done directly using OS libc interface, I don't know how fast/slow it would be on huge directories)).

This is not really patch, but rather complete new implementation (I think best idea would be actually to rename this one to FreeBsdWatcher and to patch Makefile to include it in build - maybe it would be possible to have it included in Mono's HEAD that way)

There are 2 patch files included - one for FileSystemWatcher to add necessary function to directly dispatch events, and second for changing KeventWatcher (both files are in patch-files-4.2.2.10.tar.xz archive).

I think this closes this issue once for all (and hopefully all FileWatcher related issues on FreeBSD).
Comment 8 Ivan Radovanovic 2016-01-26 00:04:26 UTC
Created attachment 166119 [details]
patch files for mono 4.2.2.10

patch files attached.
Comment 9 Ivan Radovanovic 2016-01-26 09:14:24 UTC
Created attachment 166140 [details]
patch files

I just noticed that some tests are sometimes failing (depending on system load) because I made this implementation do directory scanning *after* returning from EnableRaisingEvents (apparently MS version returns only after it is ready to detect all changes). I made our version behave the same way (although previous behavior was maybe appropriate for large number of files watched (you start watcher and while it scans directories main thread can do other tasks)).
Comment 10 joshruehlig 2016-08-19 20:34:50 UTC
Ivan/Daniel
Any reason I should use these patches vs the ones I link here?
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206593

I needed to patch up mono 4.4 to ship with the Emby plugin for FreeNAS.
Thanks,
Josh
Comment 11 Daniel Becker 2016-08-19 21:39:59 UTC
Josh, FYI, I just updated the patch in PR/206593 to mono-4.4.2.11.
Comment 12 joshruehlig 2016-08-19 21:52:01 UTC
Ok, thanks I'll stick to your patches as that is what I had applied before.

Maybe when either of you have some time to compare you could work together to produce a patch that could be accepted by mono =]
Comment 13 Walter Schwarzenfeld freebsd_triage 2018-01-12 14:58:25 UTC
Mono version is at 4.8.1.0. I don't think this is relevant anymore. Please close it.
Comment 14 joshruehlig 2018-01-12 15:25:39 UTC
I believe the patch still applies properly. I actually still use it to enable realtime media scanning for the Emby plugin that I package for the FreeNAS community. It would be great if we could get this accepted upstream, or if this got reviewed.
Comment 15 Romain Tartière freebsd_committer 2018-01-12 16:41:44 UTC
Ooops!  I understood that this change was included upstream…

Is there a PR upstream ?
Comment 16 joshruehlig 2018-01-12 16:57:36 UTC
Sorry, I confused this with https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206593.
I believe both solve the same problem, but #206593 is the patch I have been including with the Emby plugin for FreeNAS.

I does not look like there is an upstream (mono) merge request for either of these patches. I do see this github compare for #206593.
https://github.com/mono/mono/compare/mono-4.2.2.10...razzfazz:mono-4.2.2.10-freebsd_port_patch

I think it is fine to close/change the status for both of these bug reports. When I have time I could submit a new one, reference both of these bugs, CC everyone involved, and make sure the implemented changes apply to the latest mono version.
I can also submit a merge request upstream which I think is the ideal solution.
Comment 17 Ivan Radovanovic 2018-01-12 17:24:14 UTC
This was actually the most recent work I did with this watcher

https://github.com/radovanovic/monobsd/commit/27141786e3db3e5e381be812ff698d479550e678

That was when we were trying to make mono run better on FreeBSD. I am not sure if guys from .Net foundation took these patches for their watcher implementation or they did it from scratch (or they didn't do it at all). In the meantime I mostly moved away from mono, so I didn't really have time to look at this issue anymore. At point when I wrote it it passed all tests and it was able to apply cleanly to upstream. Later I realized that for my use case for .Net it was much better to have completely broken watcher implementation, so I stopped using and maintaining it...
Comment 18 Russell Haley 2018-01-12 20:20:39 UTC
My understanding is that waiting on upstream for these kinds of
patches is fruitless.

I just tested the patch to move mono to 5.2. I am willing to test if
someone puts an appropriate patch on reviews.freebsd.org and provides
test cases (e.g. name a program to test and loosely describe some test
cases I can run). FYI there will be an exp-run done for mono 5.2 once
the D13752 is approved (soonish?).

https://reviews.freebsd.org/D13752

If nobody has time please say so now and I will do my best but I'm
still trying to catch up with other projects. Once the
filesystemwatcher is tested, I can update the mono port and ask dbn@
to commit?
Comment 19 Michiel van Baak 2019-01-24 14:25:43 UTC
Hi,

One program that is easy to test is net-p2p/jackett.
On FreeBSD 12 jackett wont start because of a kevent(2) error in the FileWatcher module.
See attached log. This log is created with the latest version of Jackett port on an up-to-date FreeBSD 12 system.
mono etc are installed using pkg using the /latest repository

mono --version

root@multimedia-torrenting:~ # mono --version
Mono JIT compiler version 5.10.1.57 (5.10.1.57 Tue Dec 18 02:17:42 UTC 2018)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
        TLS:           __thread
        SIGSEGV:       altstack
        Notification:  kqueue
        Architecture:  amd64
        Disabled:      none
        Misc:          softdebug
        Interpreter:   yes
        LLVM:          supported, not enabled.
        GC:            sgen (concurrent by default)
root@multimedia-torrenting:~ #

root@multimedia-torrenting:~ # uname -a
FreeBSD multimedia-torrenting 12.0-RELEASE-p2 FreeBSD 12.0-RELEASE-p2 GENERIC  amd64
Comment 20 Michiel van Baak 2019-01-24 14:28:58 UTC
Created attachment 201376 [details]
log of jackett crashing at startup on kevent(2)
Comment 21 Michiel van Baak 2019-01-24 19:33:09 UTC
Since I'm traveling I cannot test it myself :( In three weeks I will get back home and will be able to test.
Comment 22 Michael Hunter 2019-02-20 22:49:36 UTC
(In reply to Michiel van Baak from comment #21)
Michiel,

First off let me thank you for maintaining to port for Jackett.  I also am running into issues starting it on FreeBSD12.0.  Here is the output from mono --debug JackettConsole.exe: https://pastebin.com/c8J4HB5f

Unfortunately I don't know enough to further troubleshoot this.  Were you successful in any of your testing?
Comment 23 Michiel van Baak 2019-02-21 10:18:41 UTC
(In reply to Michael Hunter from comment #22)

Hey, no problem.
Your logs show exactly the same error as I see.
My mono/kevent knowlegde is not what it should be, and I dont really know how to fix it.
Testing of this patch is on my list, but the list isn't getting smaller at the moment :(
Comment 24 Ralph Bacolod 2019-03-08 23:29:10 UTC
is the fix to the jackett issue is to upgrade the mono version in FreeBSD? 
is there an easy way to install a non-port version of Mono?
Comment 25 Ulrich Spoerlein freebsd_committer 2019-04-22 13:58:43 UTC
Ivan, the patches no longer apply cleanly (obviously). Should one try to port the attached patch from 2016 over, or start with the work under https://github.com/radovanovic/monobsd/commit/27141786e3db3e5e381be812ff698d479550e678 ?

Josh, you mentioned that you were still using the patches in 2018, did you maintain versions of them that still apply cleanly? Could you please share them, so I can test and commit them? Thanks!
Comment 26 Ivan Radovanovic 2019-04-22 20:28:20 UTC
I think I have patch for 5.10 on one computer, will try to find it these days (I think it is not too different from source in github, maybe I just didn't bother to port tests as well).

Will try to find time soon to look at this bug reported by Michiel.
Comment 27 Ivan Radovanovic 2019-05-06 20:22:53 UTC
Any pointer which version of mono patch should target?
Comment 28 Ulrich Spoerlein freebsd_committer 2019-05-07 15:03:15 UTC
If you could target the latest stable "Visual Studio" release of 5.18.1, that would be great! I'd hope that the patches would apply to most 5.x releases anyway
Comment 29 Conrad Meyer freebsd_committer 2019-05-24 01:51:22 UTC
*** Bug 206593 has been marked as a duplicate of this bug. ***
Comment 30 joshruehlig 2019-07-06 18:26:10 UTC
Ulrich,
I was using the patch from https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206593 for mono version 4.X. I have not tried updating it to 5.X but when I looked a while back it did not look like much had changed with the particular file it was touching. Maybe things are different now though.

I would be happy to test any patches that enable realtime filesystem scanning for emby using a recent version of mono.
Comment 31 Adonis 2019-08-31 22:14:52 UTC
Has a patch been made for Mono 5.x? Don't see it here.
Comment 32 kralizeck 2019-09-02 18:52:10 UTC
(In reply to Adonis from comment #31)
I think they are here:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238089
Comment 33 kralizeck 2019-09-15 10:18:00 UTC
Jackett working again on FreeBSD 12 with mono patched to version 5.20.1.34:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238089