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

AspNetCore IO Exception Invalid Arguement #8299

Closed
SarthakSoni opened this issue Aug 21, 2018 · 22 comments
Closed

AspNetCore IO Exception Invalid Arguement #8299

SarthakSoni opened this issue Aug 21, 2018 · 22 comments
Assignees

Comments

@SarthakSoni
Copy link

SarthakSoni commented Aug 21, 2018

Is this a Bug or Feature request?:

Bug

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

Microsoft.AspNetCore.Hosting

IWebHostBuilder selfHost = WebHost.CreateDefaultBuilder()
                .UseStartup<TStart>()
                .UseKestrel(options =>
                {
                    options.Listen(“127.0.0.1”, port, listenOptions =>
                    {
                        if (useHttps == true)
                        {
                            listenOptions.UseHttps(certPath, "");
                        }
                    });
                })
                .Build();

Description of the problem:

Getting IO Exception with invalid arguement in aspnet core.

at System.IO.Enumeration.FileSystemEnumerator1.FindNextEntry() at System.IO.Enumeration.FileSystemEnumerator1.MoveNext()
at System.IO.FileSystemWatcher.RunningInstance.AddDirectoryWatchUnlocked(WatchedDirectory parent, String directoryName)
at System.IO.FileSystemWatcher.RunningInstance.AddDirectoryWatchUnlocked(WatchedDirectory parent, String directoryName)
at System.IO.FileSystemWatcher.RunningInstance.AddDirectoryWatchUnlocked(WatchedDirectory parent, String directoryName)
at System.IO.FileSystemWatcher.RunningInstance.AddDirectoryWatchUnlocked(WatchedDirectory parent, String directoryName)
at System.IO.FileSystemWatcher.RunningInstance.AddDirectoryWatchUnlocked(WatchedDirectory parent, String directoryName)
at System.IO.FileSystemWatcher.RunningInstance.AddDirectoryWatchUnlocked(WatchedDirectory parent, String directoryName)
at System.IO.FileSystemWatcher.RunningInstance..ctor(FileSystemWatcher watcher, SafeFileHandle inotifyHandle, String directoryPath, Boolean includeSubdirectories, NotifyFilters notifyFilters, CancellationToken cancellationToken)
at System.IO.FileSystemWatcher.StartRaisingEvents()
at System.IO.FileSystemWatcher.StartRaisingEventsIfNotDisposed()
at System.IO.FileSystemWatcher.set_EnableRaisingEvents(Boolean value)
at Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.TryEnableFileSystemWatcher()
at Microsoft.Extensions.FileProviders.Physical.PhysicalFilesWatcher.CreateFileChangeToken(String filter)
at Microsoft.Extensions.Primitives.ChangeToken.OnChange(Func`1 changeTokenProducer, Action changeTokenConsumer)
at Microsoft.Extensions.Configuration.FileConfigurationProvider..ctor(FileConfigurationSource source)
at Microsoft.Extensions.Configuration.Json.JsonConfigurationSource.Build(IConfigurationBuilder builder)
at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()
at Microsoft.AspNetCore.Hosting.WebHostBuilder.BuildCommonServices(AggregateException& hostingStartupErrors)
at Microsoft.AspNetCore.Hosting.WebHostBuilder.Build()

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:2.1.1

@pranavkm
Copy link
Contributor

Might be a dup of aspnet/aspnet-docker#288. Can you try the workaround suggested here - aspnet/aspnet-docker#288 (comment)?

@SarthakSoni
Copy link
Author

We dont have any AddJsonFile call in our config builder.

@pranavkm
Copy link
Contributor

What does your startup code look like?

@mkArtakMSFT
Copy link
Member

@SarthakSoni please provide us with a repro project so we can investigate this issue.

@SarthakSoni
Copy link
Author

SarthakSoni commented Aug 22, 2018

IWebHostBuilder selfHost = WebHost.CreateDefaultBuilder()
                .UseStartup<TStart>()
                .UseKestrel(options =>
                {
                    options.Listen(“127.0.0.1”, port, listenOptions =>
                    {
                        if (useHttps == true)
                        {
                            listenOptions.UseHttps(certPath, "");
                        }
                    });
                })
                .Build();

@arunjvs
Copy link

arunjvs commented Aug 22, 2018

@mkArtakMSFT: We are seeing this since we upgraded from NETCORE 2.0 to 2.1. We are trying to just start a simple MVC server with Kestrel as you can see in @SarthakSoni 's snippet.

We are not hosting anything from the filesystem, why is ASP even setting up file system watchers? Is it using inotify? On what paths? This seems to be interfering with the paths owned by the widely used OMS agent on Linux. This means this fault is very likely to happen widespread on Azure Linux VMs. @SarthakSoni can you add an strace here?

@mkArtakMSFT
Copy link
Member

@SarthakSoni, can you also please share the TStart class?

@SarthakSoni
Copy link
Author

SarthakSoni commented Aug 22, 2018

class Startup
    {
        public void Configuration(IAppBuilder app)
        {
            var listener = (HttpListener)app.Properties["System.Net.HttpListener"];
            listener.AuthenticationSchemes = AuthenticationSchemes.IntegratedWindowsAuthentication;
            app.Properties["System.Net.HttpListener"] = listener;

            var config = new HttpConfiguration();
            VersionUtility.SupportedReleaseVersions = ApiConfig.SupportedVersions;
            ApiConfig.RegisterRoutes(config.Routes);
            config.MessageHandlers.Add(new ContextHandler());
            app.UseWebApi(config);
        }
    }

@pranavkm
Copy link
Contributor

  • @JunTaoLuo . I wasn't entirely aware that the CreateDefaultBuilder sets up config reload which is what would cause this issue. Like I pointed out in the earlier issue, it's likely you're running in to:

I'm guessing this means that there is a directory in your project that has illegal characters in the file or directory name. By default, the file watcher scans all subdirectories

@JunTaoLuo what would be the best way to avoid setting up config reload? @SarthakSoni \ @arunjvs you could have a look at your directory structure and see if there's something problematic there.

@JunTaoLuo
Copy link
Contributor

I have seen similar issues with config reloading (for example https://github.com/aspnet/MetaPackages/issues/282). CreateDefaultBuilder is meant as a shorthand to simplify the setting up of web hosts and it chooses many default settings, including which configurations to load/reload. If you are running into problems with how the default builder behaves, I would recommend creating your own WebHostBuilder and configuring it yourself. You should be able to copy over most of the logic at: https://github.com/aspnet/MetaPackages/blob/42b3407cd642f2d8f3f7113bff086ff3715bedfb/src/Microsoft.AspNetCore/WebHost.cs#L150-L219.

@arunjvs
Copy link

arunjvs commented Aug 23, 2018

Thanks @pranavkm and @JunTaoLuo. We are moving to our own construction based on the default template. But I would really expect a default setting just to work.

None of the parent or child directories of where our dotnet app is located has any special characters.

IMO, the problem is different, we might be observing the whole root (/) directory. And in the process we must be enumerating contents of the proc file system (/proc). All files there are not regular files, some processes (like the OMS agent) may have a set of special sockets or other file descriptors that may become accessible to other processes at times through /proc. Trying to enumerate or stat such paths may give various errors including EINVAL. This bug is that case. In general, we may want to avoid watching special filesystems like sysfs, proc and udev. We might hit similar stack traces randomly with different error codes whenever such (not so) special processes run, and we must not fail because of them.

I'd guess a repro is a straight forward - create an Azure Linux VM (with Monitoring enabled) and WebHost.CreateDefaultBuilder().Build() should just fail on 2.1 runtime.

@arunjvs
Copy link

arunjvs commented Aug 23, 2018

An strace we did confirmed above - .Build() is causing reading of tons of directories. One of the readdir_r() glibc call and the corresponding getdents() syscall failed with EINVAL when done inside an unrelated (in this case omsagent) process's /proc/${PID}/ directory.

@JunTaoLuo
Copy link
Contributor

Hmm I'll need to try repro this to see which component is enumerating the files in the root directory. I would expect the default settings configured to watch the configuration files to only spawn file watchers from within the app directory.l

@pranavkm
Copy link
Contributor

@mkArtakMSFT mkArtakMSFT assigned JunTaoLuo and unassigned pranavkm Aug 23, 2018
@JunTaoLuo
Copy link
Contributor

@SarthakSoni @arunjvs , can you provide me with a repro app? From the pieces I've seen in the issue, the default setting should not be trying to enumerate the files under /. I suspect either:

  1. The app is being executed from / which would explain the errors but this is the expected behaviour
  2. There are configuration options that is setting the app's content root to /. If we are doing this then there's a bug, but I haven't seen any indication that's suggesting that this is the case.

@davidfowl
Copy link
Member

You shouldn't need to make your own version of CreateDefaultBuilder, you can clean configuration providers if they are causing problems.

        public static IWebHostBuilder CreateWebHostBuilder(string[] args) =>
            WebHost.CreateDefaultBuilder(args)
                .ConfigureAppConfiguration(builder => 
                {
                    builder.Sources.Clear();
                })
                .UseStartup<Startup>();

@avertes Are you hosting your ASP.NET Core application at the root of your drive?

@arunjvs
Copy link

arunjvs commented Aug 24, 2018

@JunTaoLuo: 1 is not the case if you asking about the location of the assemblies (i.e. the publish output). But does it look at Environment.CurrentDirectory? That would be a problem since we do not want to control or change it. If that is not the case, I suspect it's 2 either due to a bug or a specific file system layout or mount graph, though we did nothing special to the default file system layout of the distro. I will follow up with you offline for source sharing if you think Environment.CurrentDirectory is not the culprit.

@davidfowl: I'd guess building it ourselves instead of undoing the defaults is cleaner anyway - we've realized there's lot of extra bloat in the default builder (in our project's context) that we don't need anyway.

@davidfowl
Copy link
Member

@davidfowl: I'd guess building it ourselves instead of undoing the defaults is cleaner anyway - we've realized there's lot of extra bloat in the default builder (in our project's context) that we don't need anyway.

Maybe, I'd like to understand what bloat there is. We made it a point to make everything possible to undo, which also makes it possible to get additional features as they were added. You do have full control however and if you want to rewrite the method thats completely fine.

@arunjvs
Copy link

arunjvs commented Aug 28, 2018

@davidfowl We needed really less - a new WebHostBuilder() and UseKestrel() primarily. It's bloat only for our immediate project as I have stated.

The primary reason we were using the default was indeed to automatically get additional features/improvements, plus that's what the docs also suggest. But because of this bug, we are constructing ourselves, and it appears to be a simpler and shorter code than undoing the defaults.

The key point here is that a simple runtime upgrade broke the default from working correctly in our situation, so we doing it ourselves in these situations sounds safer in general, since those 'additional features' may not always work as expected.

@davidfowl
Copy link
Member

@davidfowl We needed really less - a new WebHostBuilder() and UseKestrel() primarily. It's bloat only for our immediate project as I have stated.

The primary reason we were using the default was indeed to automatically get additional features/improvements, plus that's what the docs also suggest. But because of this bug, we are constructing ourselves, and it appears to be a simpler and shorter code than undoing the defaults.

Sounds fine to me!

@mkArtakMSFT
Copy link
Member

Guys, is there any other pending action in here?
/cc @davidfowl, @JunTaoLuo

@davidfowl
Copy link
Member

Nope

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

6 participants