MerlinVR / UdonSharp

An experimental compiler for compiling C# to Udon assembly
MIT License
669 stars 88 forks source link

[UdonSharpAssetCompileWatcher.cs] Crash on macOS #22

Closed owlboy closed 4 years ago

owlboy commented 4 years ago

Describe the bug in detail: Unity crashes very quickly after UdonSharp is imported into a project on macOS.

When reloading the project Unity promptly crashes too.

Removing UdonSharp returns the project to normal.

Provide steps/code to reproduce the bug: Load UdonSharp into a project on macOS.

Expected behavior: No crash 😅.

Additional Information: I think it has to do with filesystem access/number of open files/maybe permissions.

I am using APFS (Encrypted) and it is not case sensitive. I'm not sure if that is relevant.

The comment on line 76 of UdonSharpAssetCompileWatcher.cs is interesting.

This appears to be the relevant section of the macOS crashlog:

IOException: kqueue() error at init, error code = '24'
  at System.IO.KqueueMonitor.Start () [0x00046] in <d2957de1c3fd4781a43d89572183136c>:0 
  at System.IO.KeventWatcher.StartDispatching (System.IO.FileSystemWatcher fsw) [0x00033] in <d2957de1c3fd4781a43d89572183136c>:0 
  at System.IO.FileSystemWatcher.Start () [0x00000] in <d2957de1c3fd4781a43d89572183136c>:0 
  at System.IO.FileSystemWatcher.set_EnableRaisingEvents (System.Boolean value) [0x00014] in <d2957de1c3fd4781a43d89572183136c>:0 
  at (wrapper remoting-invoke-with-check) System.IO.FileSystemWatcher.set_EnableRaisingEvents(bool)
  at UdonSharp.UdonSharpAssetCompileWatcher.SetupWatchers () [0x00062] in /Users/USER/GitHub/greatpug2018udon/assets/UdonSharp/Editor/UdonSharpAssetCompileWatcher.cs:51 
  at UdonSharp.UdonSharpAssetCompileWatcher.OnEditorUpdate () [0x00001] in /Users/USER/GitHub/greatpug2018udon/assets/UdonSharp/Editor/UdonSharpAssetCompileWatcher.cs:168 
  at (wrapper delegate-invoke) <Module>.invoke_void()
  at UnityEditor.EditorApplication.Internal_CallUpdateFunctions () [0x00010] in /Users/builduser/buildslave/unity/build/Editor/Mono/EditorApplication.cs:200

Versions: macOS 10.15.4, Unity 2018.4.20f1

owlboy commented 4 years ago

If Unity uses the version/install of mono installed by VisualStudio, then here is the version I have installed:

$ mono --version
Mono JIT compiler version 6.8.0.123 (2019-10/1d0d939dc30 Thu Mar 12 23:19:08 EDT 2020)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
    TLS:           
    SIGSEGV:       altstack
    Notification:  kqueue
    Architecture:  amd64
    Disabled:      none
    Misc:          softdebug 
    Interpreter:   yes
    LLVM:          yes(610)
    Suspend:       hybrid
    GC:            sgen (concurrent by default)
MerlinVR commented 4 years ago

Unity uses its own version of mono afaik. Can you try increasing your open file limit and see if that fixes the issue? https://krypted.com/mac-os-x/maximum-files-in-mac-os-x/

owlboy commented 4 years ago

When brought into an empty project (+VRCSDK3), the crash does not appear to happen.

Is this really just a matter of too many files being opened at once? You just replied as I was typing this!

owlboy commented 4 years ago

Why do thousands of files have to be open?

orels1 commented 4 years ago

It is a system wide limit

owlboy commented 4 years ago

That blog post is 11+ years old. It states things like 12,288 10,240 as defaults.

My system is already configured for:

$ sysctl -A | grep kern.maxfiles
kern.maxfiles: 98304
kern.maxfilesperproc: 49152
owlboy commented 4 years ago

@orels1 Why does UdonSharp need so many open*?

MerlinVR commented 4 years ago

UdonSharp doesn't, it will only open .cs files in your project. If you manage to have 10's of thousands of unique scripts in a project I suppose it could cause issues. But that's not a realistic use case at the moment.

orels1 commented 4 years ago

Udon sharp just registers another watcher though. You can hit that limit just in a very large unity project if you have extra watchers (Lyuma had to set their limit to 100000 as they were hitting it in just an avatar project). It still registers under the main unity process from what i understand. Hence why you're not hitting it in the empty project outright

orels1 commented 4 years ago

My drive isn't encrypted though, but i can't see that being the issue. I have a per process file limit set to 100k as well.

owlboy commented 4 years ago
image

Is this unusual? I admit The Pug project is vast with lots of stuff in it.

Edit: Half of this is just Amplify with 444 .cs files contained within /assets/AmplifyShaderEditor/

MerlinVR commented 4 years ago

Not particularly unusual if you have a ton of external libraries, but that shouldn't make you hit the file limit immediately.

orels1 commented 4 years ago

Its hard to say what else is in the project, i think its just a cumulative effect of everything combined. UdonSharp just was an extra bit on top of a lot of files already watched by unity. At least that's not something unusual to happen. I hit that limit when adding an extra hot reloading extension to vscode for example that tried to register extra watchers on the same set of files as vscode itself under the vscode process which instantly tripped the limit.

owlboy commented 4 years ago

Does it need to open lots of files initially during the project load to look for just the .cs files? Is there potentially a burst of open files for a moment that it can not get past in my case?

MerlinVR commented 4 years ago

Do you also get an error System.IO.IOException: Too many open files after the kqueue error? Usually the errors that are due to the file limit will also print that, at least going by similar reports.

owlboy commented 4 years ago

@Merlin-san No, but… 👀 when I tried increasing that 1024 I saw to 2048 I did see that in the log. And my system hung hard. (I know that was silly to try) I think it may crash first.

The next lines when no modifications are made, a failure to load a file:


(Filename: <d2957de1c3fd4781a43d89572183136c> Line: 0)

Could not open file /Applications/Unity/Hub/Editor/2018.4.20f1/Unity.app/Contents/Resources/unity editor resources for read
MerlinVR commented 4 years ago

👀 that looks like it comes from another system as a result of running out of file handles. The asset compile watcher will only ever touch files under the Assets directory.

owlboy commented 4 years ago

Update! I managed to import it into The Pug project without it crashing!

These are from the console:

IOException: kqueue() error at init, error code = '24'
System.IO.KqueueMonitor.Start () (at <d2957de1c3fd4781a43d89572183136c>:0)
System.IO.KeventWatcher.StartDispatching (System.IO.FileSystemWatcher fsw) (at <d2957de1c3fd4781a43d89572183136c>:0)
System.IO.FileSystemWatcher.Start () (at <d2957de1c3fd4781a43d89572183136c>:0)
System.IO.FileSystemWatcher.set_EnableRaisingEvents (System.Boolean value) (at <d2957de1c3fd4781a43d89572183136c>:0)
(wrapper remoting-invoke-with-check) System.IO.FileSystemWatcher.set_EnableRaisingEvents(bool)
UdonSharp.UdonSharpAssetCompileWatcher.SetupWatchers () (at assets/UdonSharp/Editor/UdonSharpAssetCompileWatcher.cs:51)
UdonSharp.UdonSharpAssetCompileWatcher.OnEditorUpdate () (at assets/UdonSharp/Editor/UdonSharpAssetCompileWatcher.cs:168)
UnityEditor.EditorApplication.Internal_CallUpdateFunctions () (at /Users/builduser/buildslave/unity/build/Editor/Mono/EditorApplication.cs:200)
IOException: Too many open files
System.IO.Directory.InsecureGetCurrentDirectory () (at <e1319b7195c343e79b385cd3aa43f5dc>:0)
System.IO.Directory.GetCurrentDirectory () (at <e1319b7195c343e79b385cd3aa43f5dc>:0)
System.Environment.get_CurrentDirectory () (at <e1319b7195c343e79b385cd3aa43f5dc>:0)
Better.BuildInfo.BuildInfoSettings.DsWJD0G5F7Ezv3k3ioEMm$Y (System.String ) (at <db62f3042cf341bea1e9d40279ec9ada>:0)
Better.BuildInfo.BuildInfoSettings.get_ReportToOpen () (at <db62f3042cf341bea1e9d40279ec9ada>:0)
Better.BuildInfo.BuildInfoProcessor+PAUY_0jon17FAm7rPh3Z6NepbKMu6gnyCd6f2Erpj7YJgXpep5w9zL6F0Oya0arscw.k43846Wzp1hfFxBcpW9drM8 () (at <db62f3042cf341bea1e9d40279ec9ada>:0)
UnityEditor.EditorApplication.Internal_CallUpdateFunctions () (at /Users/builduser/buildslave/unity/build/Editor/Mono/EditorApplication.cs:200)

Edit: Gonna remove BetterBuildInfo now.

owlboy commented 4 years ago

Bad things are still happening. I got a crash after looking around a bit.

MerlinVR commented 4 years ago

That still seems like an issue that should be fixable by increasing the max open file limit. Did you increase it or just leave the limit? For reference some individual applications suggest a limit of 200k, if you have a bunch of applications that open files it's totally possible to run out even if your limit is 100k.

owlboy commented 4 years ago

I did not change the limit yet.

If I change *.cs in both places to *.merlin the issue does not happen.

This means I am struggling with just 837 .cs files or so. I think. And half of it is Amplify. This does not square with having to increase the open files limit to me.

This is also after a fresh boot with only Unity and a few other standard things open.

While Unity is open right now with this *.merlin in place, it appears only 353 files are open if I inspect the Unity process. With *.cs in place, I get the error, and then I see around 1614 files open.

I don't think I have over 98,304 or 49,152 files open.

MerlinVR commented 4 years ago

Can you increase the limit? The only place that UdonSharp will have those files open is the asset compile watcher and there's not much I can do on that front without re-implementing the C# FileSystemWatcher primitive. If you have that many files open it's very likely it's some other utility and the files open by UdonSharp are pushing you over that limit. This is just debugging, if you increase the limit and the problem is solved then something is eating all your file handles, and if it's not then it's a bug somewhere else.

MerlinVR commented 4 years ago

Changing the file extensions on all scripts will just make the file watcher not watch them so it's hiding the root issue.

owlboy commented 4 years ago

Ok, done. Half a million:

$ sysctl -A | grep kern.maxfiles
kern.maxfiles: 524288
kern.maxfilesperproc: 524288

Same result:

IOException: kqueue() error at init, error code = '24'
System.IO.KqueueMonitor.Start () (at <d2957de1c3fd4781a43d89572183136c>:0)
System.IO.KeventWatcher.StartDispatching (System.IO.FileSystemWatcher fsw) (at <d2957de1c3fd4781a43d89572183136c>:0)
System.IO.FileSystemWatcher.Start () (at <d2957de1c3fd4781a43d89572183136c>:0)
System.IO.FileSystemWatcher.set_EnableRaisingEvents (System.Boolean value) (at <d2957de1c3fd4781a43d89572183136c>:0)
(wrapper remoting-invoke-with-check) System.IO.FileSystemWatcher.set_EnableRaisingEvents(bool)
UdonSharp.UdonSharpAssetCompileWatcher.SetupWatchers () (at assets/UdonSharp/Editor/UdonSharpAssetCompileWatcher.cs:51)
UdonSharp.UdonSharpAssetCompileWatcher.OnEditorUpdate () (at assets/UdonSharp/Editor/UdonSharpAssetCompileWatcher.cs:168)
UnityEditor.EditorApplication.Internal_CallUpdateFunctions () (at /Users/builduser/buildslave/unity/build/Editor/Mono/EditorApplication.cs:200)

Edit: Just to be clear, I was editing *.cs in the script. Not the filenames. My goal was to see if the script was indeed "opening" all files in the project instead of just the intended ones.

owlboy commented 4 years ago

This appears to work:

string[] directories = Directory.GetDirectories("Assets/UdonSharpQuarantine", "*", SearchOption.AllDirectories).Append("Assets/UdonSharpQuarantine/").ToArray();
MerlinVR commented 4 years ago

That's also reducing the scope of the search so it's hitting fewer scripts and hiding the issue. So that's not a solution I can implement without breaking the file updating for everyone else. If it's still happening with a raised file limit, it's probably just a bug in mono that has been reported and fixed in more recent versions https://github.com/OmniSharp/omnisharp-vscode/issues/3041. Unity is using mono version 5.11.0 which is in the range of versions that people have reported the issue.

Can you try going to Edit > Project Settings > Udon Sharp and disabling the Auto compile on modify check box? That will prevent the file watchers from raising events so they shouldn't be opening the files.

owlboy commented 4 years ago

I also added all of Amplify in that quarantine folder, and it is happy with that.

owlboy commented 4 years ago

No errors with Auto-compile on modifying unchecked.

MerlinVR commented 4 years ago

Then that's the workaround for now, though you'll need to click the "Compile All UdonSharp Programs" button when you update a script. I don't think I can do much to fix it aside from re-implementing FileSystemWatcher since upgrading mono is not viable for Unity, and even then it might run into similar problems with a custom implementation. I'll keep note of the issue, but a re-implementation of FileSystemWatcher would be decent chunk of work to do "right" since it wraps OS events for file changes which I can't do reliably myself. And when it comes to messing with filesystem operations it's likely to cause issues for other people if done incorrectly.

owlboy commented 4 years ago

Are you open to adding an optional preference to only inspect a specified folder as an alternative to the whole /assets/ dir? This seems like it would be an OK compromise. And if your concerns about the CPU usage are true, then it would help there too. And if someone finds there is a significant impact for them they could also use the option.

This is what I was trying to show with the Quarantine. I know it limited the scope.

Edit: Later in the Discord we discussed a directory blacklist instead. 👍

MerlinVR commented 4 years ago

Yeah I'll add an option for specifying a folder that's scanned. Though that's more of a band-aid and if you're hitting errors regardless of the file cap I can't be certain if that fix will continue to work for you consistently, or at what point it will break.

owlboy commented 4 years ago

That makes sense, it's definitely a workaround. Thank you!

If all my testing is correct, I just need to stay under 444 UdonSharp.cs files per project with the bandaid 😀.

owlboy commented 4 years ago

I looked at the top paid and free assets along with assets recommended by VRChat on the Asset Store and made this list:

Proposed starter internal blacklist for popular asset store assets /assets/AmplifyShaderEditor/ /assets/AmplifyImpostors/ /assets/Bakery/ /assets/DynamicBone/ /assets/Heureka/ <-- Asset Hunter Pro /assets/MeshBaker/ /assets/Oculus/ /assets/Pavo Studio/ <-- Muscle Animation Editor (Recommended by VRChat) /assets/Procedural Worlds/ <-- Gaia /assets/SteamVR/ /assets/SuperCombiner/ <-- SuperCombiner (Recommended by VRChat) /assets/unity-chan!/

/assets/Plugins/GitHub/ <-- GitHub for Unity /assets/Plugins/RootMotion/ <-- Final IK /assets/Plugins/Sirenix/ <-- Odin

/assets/Standard Assets/

Are there reasons to not blacklist these by default too? /assets/Editor/ /assets/Plugins/

/assets/Udon/ /assets/VRChat Examples/ /assets/VRCSDK/

MerlinVR commented 4 years ago

Mitigated in f98ed555a1c6afa7d88b03ad476e102e492d0cec which is included in https://github.com/Merlin-san/UdonSharp/releases/tag/v0.15.10 the proper fix requires a mono update from Unity's side. The default blacklist chosen for the moment is:

"Assets/Udon/Editor/",
"Assets/Udon/Serialization/",
"Assets/VRChat Examples/",
"Assets/VRCSDK/Dependencies/",
"Assets/UdonSharp/Editor/",
// Common 3rd party editor assets
"Assets/AmplifyShaderEditor/",
"Assets/AmplifyImpostors/",
"Assets/Bakery/",
"Assets/Editor/x64/Bakery/",
"Assets/Procedural Worlds/", // Gaia
"Assets/Pavo Studio/", // Muscle editor
"Assets/Plugins/RootMotion/", // FinalIK

This list is subject to change, you can add other folders to the blacklist in the Udon Sharp project settings tab.