Runtime: dotnetcore on linux reads assembly contents, even if loaded

[jeffschwMSFT]

Moved from https://github.com/dotnet/core-setup/issues/5205

@force-net commented on Tue Feb 19 2019
We're using .netcore on linux and using next deploy algorithm: update content of folder with dll's and restart application. It is impossible on windows (because dll's is locked), but works fine on linux at .net core 2.0. When we've switched to 2.1, this workflow become unusable due sporadic crashes when dll's are updated for running application. 2.2 version has same issue.
I've found that this problem is related to exceptions (stackframes) or reflection. Simple app reproduces this behavior.
Steps to reproduce
Create a simple console app with this code:
C# class Program { static void Main(string[] args) { while (true) { try { System.Threading.Thread.Sleep(1000); throw new System.InvalidOperationException(); } catch (System.Exception e) { System.Console.WriteLine(e); } } } }
publish it and deploy to Linux system (Ubuntu 16.04 and 18.04 was checked)
Run it dotnet ConsoleApp1.dll
Edit ConsoleApp1.dll by removing some symbol in a dll body
Expected behavior
Nothing changed, application continue working
Actual behavior
Crash with Segmentation fault (core dumped) or BadImageFormatException or no string associated with token or something else.
Environment data
Host (useful for support):
Version: 2.2.2
Commit: a4fd7b2c84

.NET Core SDKs installed:
No SDKs were found.

.NET Core runtimes installed:
Microsoft.NETCore.App 2.2.2 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

cc @janvorli

[mikedn]

Presumably "reads" in this case actually means "opens". It should be fine for the runtime to read assembly contents from an existing file descriptor as many times as it needs, even after the file was replaced. So the problem might be that the runtime actually opens and reads the new file.

If that the case, this may happen on Windows too. While it doesn't allow you to delete loaded dll files it allows you rename them or move them to another folder on the same volume and then you can copy the new dll.

[sdmaclea]

/cc @lpereira

[lpereira]

There's no easy way to fix this.

While the MAP_PRIVATE flag prevents changes to mappings from going to the backing file by employing CoW, it doesn't protect you from the file being changed under the rug; changes made in the file may or may not be visible in the memory area.

We might get away with this by mapping the files with PROT_WRITE, and doing something like mem[0] = mem[0], and then calling mprotect(2) on them with PROT_READ for instance, just to trigger the CoW; this would probably need to be done every PAGE_SIZE bytes, unless MAP_HUGETLB has been specified. The disadvantage of this is that this will significantly increase memory usage when there are lots of applications using the same assemblies in the same system.

Another way to solve this problem is to notify the application that it's going to be restarted. When this happens, it can delete its own DLLs (processes that have these files opened will continue to be able to access the data but those files can't be found anymore; the last one to turn off the lights will actually trigger file deletion), so that the new ones can be copied over. They can be copied with the same name. Now, when the application restarts, they'll pick up the new files. The disadvantage of this is that the application -- or the runtime -- would have to be "hot-reload-aware"; I don't know if it is already.

[sdmaclea]

So

• the process has a handle to the opened assembly.
• The assembly is memory mapped into the process.
• If it is modified by another process, the modifications will be visible in the runtime.
• However if the file is deleted, the delete would not be visible to the runtime (with an open handle)
• However if the file was atomically replaced, the change would not be visible to the runtime (with an open handle)

The bug report talks about

Edit ConsoleApp1.dll by removing some symbol in a dll body

w/o a SDK

.NET Core SDKs installed:
No SDKs were found.

The bug report at top is not a supported scenario.

The corect solution is to use rename to overwrite the file. As described in Stackoverflow https://unix.stackexchange.com/questions/24395/rewrite-existing-file-so-that-it-gets-replaced-by-new-version-atomically-only-o

What you're describing sounds exactly like a basic rename to overwrite a file.

When you rename/move one file on top of another, the old file is unlinked. Meaning the 
file still exists, but it isn't in the filesystem tree any more. Thus old applications will 
continue to be able to access the file as long as they keep it open. Once all 
applications have closed the old file, then its actually unallocated on the disk.

The rename system call is an atomic operation. So to do this you would create a new file
under a different name, and then call rename to rename the temporary file as the one 
you want to replace. Since the operation is atomic, there is absolutely no period where 
the file is missing. It instantly goes from old file to new file.

It is possible you should actually be reporting an issue with the SDK and its process of updating assemblies.

[sdmaclea]

@force-net Closing this issue as this is not likely a runtime issue.

[force-net]

Ok. I've understood problem with correct handling of such situation, but may be runtime should handle it in someway also? E.g. crash with specific exception (currently - random crash is occurred here, so it is hard to determine real issue if you do not know this situation).

Also, if it possible runtime can catch an internal problem and continue to work without some features (e..g. without stacktrace like 2.0 runtime).

As I understand, fully correct way of updating running application is:

1. Stop application
2. Copy new files
3. Run application

But it can be replaced with some unusual scheme:

1. Delete all files in application directory
2. Copy new files
3. Restart application

[sdmaclea]

@force-net
What are you actually doing? I suspect the initial description above is actually incomplete. It says you are editing the assembly without an SDK. How/why are you doing that?

[sdmaclea]

@force-net If you were actually building on machine.

You could have a softlink in the path to the live (most recent version)
Each time you build you could publish to a unique address.

Something like this could work if building in bash.

MyVersion=myApp_$(date -Iseconds)
dotnet publish --output MyPath/$(MyVersion)
# Create a soft link so we can atomically mv it later
ln -s MyPath/$(MyVersion) MyAppDryRun
# Optionally test
# Atomically update the soft link to the latest build
mv -T MyAppDryRun MyPath/MyAppLatest

Obviously if you were uploading a new version via some other means a similar strategy could be used...

[sdmaclea]

@janvorli I believe the functional change from 2.0.0 to 2.1 may have occurred due to dotnet/coreclr#16752 (Where we removed the advisory flock calls).

[sdmaclea]

I reopened this issue. If we can restore the advisory locks w/o degrading functionality we should consider it. While the advisory locks do not guarantee anything, they do allow well written programs to behave correctly.

Opinions @jkotas @janvorli ? Is is possible to detect systems which do not support flock?

[sdmaclea]

The original code treated failure to lock as an error. (And didn't handle EINTR which might not occur if LOCK_NB is set)

    int   lock_mode = LOCK_SH;

    // While the lock manager is able to provide support for share modes within an instance of
    // the PAL, other PALs will ignore these locks.  In order to support a basic level of cross
    // process locking, we'll use advisory locks.  FILE_SHARE_NONE implies a exclusive lock on the
    // file and all other modes use a shared lock.  While this is not as granular as Windows,
    // you can atleast implement a lock file using this.
    lock_mode = (dwShareMode == 0 /* FILE_SHARE_NONE */) ? LOCK_EX : LOCK_SH;

    if(flock(filed, lock_mode | LOCK_NB) != 0) 
    {
        TRACE("flock() failed; error is %s (%d)\n", strerror(errno), errno);
        if (errno == EWOULDBLOCK) 
        {
            palError = ERROR_SHARING_VIOLATION;
        }
        else
        {
            palError = FILEGetLastErrorFromErrno();
        }

        goto done;
    }

If we simply ignored most errors would we work on filesystems which didn't support locking?

    // While the lock manager is able to provide support for share modes within an instance of
    // the PAL, other PALs will ignore these locks.  In order to support a basic level of cross
    // process locking, we'll use advisory locks.  FILE_SHARE_NONE implies a exclusive lock on the
    // file and all other modes use a shared lock.  While this is not as granular as Windows,
    // you can atleast implement a lock file using this.
    int lock_mode = (dwShareMode == 0 /* FILE_SHARE_NONE */) ? LOCK_EX : LOCK_SH;

    if(flock(filed, lock_mode | LOCK_NB) != 0) 
    {
        TRACE("flock() failed; error is %s (%d)\n", strerror(errno), errno);
        if (errno == EWOULDBLOCK) 
        {
            palError = ERROR_SHARING_VIOLATION;
            goto done;
        }
    }

[force-net]

@ sdmaclea

What are you actually doing? I suspect the initial description above is actually incomplete. It says you are editing the assembly without an SDK. How/why are you doing that?

We have production application (with a lot dll's and dependencies), which updates in next scheme: we're copying over new version and after copying complete restart app. These operations can be performed externally, without direct attaching to target system (files can be copied over SAMBA, scp or rsync, app can be restarted by itself in most suitable moment), also restart was very fast.
Currently, due this problem we're switched to more robust way: copy new version to separate directory, stop application, update files locally, start application.

Production server has only .netcore runtime, SDK is not required for it.

My example with dll editing is compact example to show problem. In our situation - new version of dll causes crash of application due another location of metadata or something else.

[sdmaclea]

We have production application (with a lot dll's and dependencies)

When there are multiple dll's and dependencies, it is hard to reason about which combination of dll's will work. IT is better to replace the entire set atomically.

In part this is because the .NET runtime is allowed to open any of the dependencies at any time. If in the middle of your upgrade any of the files are incompatible or only partially written it will cause problems.

we're switched to more robust way

Your new approach is preferred.

[force-net]

Yep, this is not ideal way to update from our side, but .net core can peform some checks at least for more stable exception to throw to recognize such situation in future (like googling exception text for documentation).
Also, is it possible to load all related assemblies into memory (e.g. with Assembly.Load(byte[]) from our side to be convinced that we do not use files, stored at disk anymore? Or related file will be used for metadata reading anyway.

Also, code is loaded into memory anyway, so, problem with metadata only, as I understood, so, it affect only reflection and exception logging. Am I correct? Is it serious memory impact to load metadata into memory?

[mikedn]

Maybe I'm missing something but why is this an issue? You'd have the same problem updating any binary executable file that is opened in a program (.so/.dll or anything similar). Traditionally this are mapped into memory and read on demand, not read ahead at the time of the load. That would be quite horrible in terms of performance.

[sdmaclea]

Use of cooperative flock was disabled in dotnet/coreclr#16752 because of problems on some filesystems. There is no documentation in the original issue to describe the actual affected filesystems.

Without sufficient knowledge of the affected filesystems it is difficult to determine whether this can safely be reenabled.

If someone were to consider reenabling this they would need to test a matrix of combinations including:

• various local filesystem ext4, ext3, ntfs, fat, tmpfs, fuse, sshfs...
• various network filesystems NFS, Samba/CIF
• read-only filesystems
• various crash recovery scenarios (are locks properly cleared on crashes)

I am recommending will not fix at this time

[sdmaclea]

There is no way for the .NET runtime to determine this is happening.

Removed the bug label as this appears to be functioning as currently intended.

[sdmaclea]

be convinced that we do not use files, stored at disk anymore?

The easiest way would be to copy the files into a new directory which is readonly at startup, then you know none of the original files are being used. This is effectively what you are already doing.

not ideal way to update from our side

Robust is ideal.

If performance is your concern, you can move the uploaded directory instead of the files.
If scripting is your concern you can simply do this every time at startup.

[janvorli]

The flock was removed based on problems that a large customer had with running .NET Core when assemblies were located on Andrew File System (https://en.wikipedia.org/wiki/Andrew_File_System).