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

Fix a stack overflow in non-writable working directories #129

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

DavidL344
Copy link
Contributor

Fixes #128. By using the running application's directory as the base path, the package won't access possibly non-writable working directories.

Video_2024-02-26_16-36-59.mp4

By using the running application's directory as the base path, the package won't access possibly non-writable working directories
@mayuki
Copy link
Owner

mayuki commented Mar 5, 2024

Thanks for the PR!

It appears that the problem occurs in combination with the default registered configuration provider.
I would like to investigate this in detail.

@mayuki
Copy link
Owner

mayuki commented Mar 7, 2024

It looks good to have ContentRoot in the same directory as the executable file.
By the way, I couldn't reproduce it in my environment (Devcontainer), but could you tell me about your filesystem and kernel/OS?

@DavidL344
Copy link
Contributor Author

Sure thing! Here are the specs of the system I could reproduce the issue on:

  • OS: Ubuntu 22.04 LTS (x64)
  • Kernel: linux 6.5.0-21-generic
  • Filesystem: ext4

The issue also occurs on the Ubuntu Live CD as well - although the stack doesn't overflow there, the app still runs with a delay under previously mentioned directories (including the root path).

If you'd like to try reproducing the issue on the Ubuntu Live CD, this is a script I use to install .NET 8 under a live environment:

#!/bin/bash

# Resources:
# https://learn.microsoft.com/en-gb/dotnet/core/install/linux
# https://learn.microsoft.com/en-gb/dotnet/core/install/linux-ubuntu#register-the-microsoft-package-repository

sudo apt update
sudo apt install wget -y

# Get Ubuntu version
declare repo_version=$(if command -v lsb_release &> /dev/null; then lsb_release -r -s; else grep -oP '(?<=^VERSION_ID=).+' /etc/os-release | tr -d '"'; fi)

# Download Microsoft signing key and repository
wget https://packages.microsoft.com/config/ubuntu/$repo_version/packages-microsoft-prod.deb -O ~/Downloads/packages-microsoft-prod.deb

# Install Microsoft signing key and repository
sudo dpkg -i ~/Downloads/packages-microsoft-prod.deb

# Clean up
rm ~/Downloads/packages-microsoft-prod.deb

# Update packages
sudo apt update

# Install .NET 8.0 SDK
sudo apt install dotnet-sdk-8.0 -y

What I haven't accounted for is a scenario where the application could be installed and run from a read-only directory - that would mean the application's startup time would still be slow if the it were to be run from its installed path. Maybe a check for read-write permissions in the working directory (and changing the ContentRoot to a read-writable one if the current one is not) could solve the problem!

@DavidL344
Copy link
Contributor Author

There's one thing I've completely missed - I wasn't really aware what the FileSystemWatcher does, but I've noticed that the reason why it's slower in certain directories and faster in others is not whether or not they're read-only, but rather the amount of content they have in them.

According to #132, the FileSystemWatcher in this case recursively lists all files in the current directory, which in this case, is the running application's working directory. By changing the working directory to where the application is running from, there's a higher chance of the directory not containing as many files, which speeds up the application's startup time, since the FileSystemWatcher lists less files.

So, what my fix essentially does, is it always makes the FileSystemWatcher recursively read files from the directory the application is stored in, which makes the startup time the same from everywhere. And since that directory doesn't have nearly as many files as other ones the app might be running from (like the home directory, or even worse, the root directory, which makes the app list the entire filesystem's contents before running), I believe this could be a decent fix for now, at least before any other PR addressing this issue in a better way is created.

Are there any changes I should make before the PR could be considered ready? Thank you!

@mayuki mayuki merged commit 337647f into mayuki:master Jun 23, 2024
3 checks passed
@mayuki
Copy link
Owner

mayuki commented Jun 23, 2024

Apologies for the delay, but I have merged it. Thanks!

@GGG-KILLER
Copy link

@mayuki would it be possible to release a new nuget version with this fix?

I'd like to stop using my solution from #132 as that's a lot of unnecessary boilerplate.

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

Successfully merging this pull request may close these issues.

The stack overflows when running from a non-writable path
3 participants