Weird binary file lock with PublishSingleFile=true

[JustArchi]

Background

Hello.

I have a quite unusual scenario where my application would like to move its own binary while running. On linux, I'm free to do anything I want with currently running binary, including moving it. On Windows, at least usually, I'm not able to delete the running binary, but I'm still able to move it, which satisfies my usage.

Issue

I've noticed inconsistency in regards to this when using PublishSingleFile=true. Ideally I'd like to keep being able to move the binary away, which I use right now with no further issues.

In particular, this is what happens (only on Windows):

• When you launch a PublishSingleFile=true project for the first time, or after you've manually cleaned %TEMP%\.net, the app unpacks itself and you're free to rename/move the binary as you please.
• However, when you launch that project again, with the content already extracted, you're no longer able to do anything with the original binary, including moving/renaming it. The binary now holds exclusive file lock to itself.

If anything, I'd expect this behaviour to be consistent, or logically, more restrictive when we unpack for the first time, rather than when re-using existing structure. Instead, it's entirely different, to the point that makes me believe it's something worth reporting or getting more details about.

Of course, judging by the above, the "workaround" for my use case is manually cleaning %TEMP%\.net after each program run, which is a very weird solution, but a working one. I'm not entirely sure why the application would hold exclusive file access when re-using the existing structure but not when unpacking for the first time, which is exactly why I've decided to create this issue.

Ideally it'd be nice if that lock wasn't needed (so I could keep using my planned use case), but in case you actually have reasoning for it, you'd likely want to keep it up for both cases and not only one that is effectively a subset of operations (run) in another one (unpack and run). In any case, at least in my opinion, it's something worth investigating and deciding whether that exclusive file lock could be released (like when run for the first time), or if it's actually needed in both cases (in which case you might want to add it to first run as well).

Thank you in advance for your time in regards to this isssue.

.NET Core SDK (reflecting any global.json):
 Version:   3.1.100
 Commit:    cd82f021f4

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support):
  Version: 3.1.0
  Commit:  65f04fb6db

[swaroop-sridhar]

This issue should be moved to the https://github.com/dotnet/runtime repo.

The runtime host doesn't explicitly take a lock on itself.
In fact, if the extraction is already available, it only does less work: https://github.com/dotnet/runtime/blob/master/src/installer/corehost/cli/apphost/bundle/runner.cpp#L53-L62

[Logerfo]

@swaroop-sridhar thanks for the link.
Looking at #6576, I can see that the behavior on Linux and Windows are different.
Check out the full context:

runtime/src/installer/corehost/cli/apphost/bundle/runner.cpp

Lines 42 to 64 in bcd203d

	map_host();
	reader_t reader(m_bundle_map, m_bundle_length);
	// Read the bundle header
	reader.set_offset(marker_t::header_offset());
	header_t header = header_t::read(reader);
	extractor_t extractor(header.bundle_id(), m_bundle_path);
	m_extraction_dir = extractor.extraction_dir();
	// Determine if embedded files are already extracted, and available for reuse
	if (extractor.can_reuse_extraction())
	{
	return StatusCode::Success;
	}
	manifest_t manifest = manifest_t::read(reader, header.num_embedded_files());
	extractor.extract(manifest, reader);
	unmap_host();
	return StatusCode::Success;

Note that map_host is always called, but unmap_host is only called when the extraction cannot be reused. Still doesn't tell why the behavior is different depending on which OS you're using, but we'll get there.
As you can see below, the map_host function calls pal::map_file_readonly:

runtime/src/installer/corehost/cli/apphost/bundle/runner.cpp

Lines 15 to 17 in bcd203d

	void runner_t::map_host()
	{
	m_bundle_map = (int8_t *) pal::map_file_readonly(m_bundle_path, m_bundle_length);

This map_file_readonly function has multiple source codes.
For Unix:

runtime/src/installer/corehost/common/pal.unix.cpp

Lines 60 to 89 in 1d5123f

	void* pal::map_file_readonly(const pal::string_t& path, size_t& length)
	{
	int fd = open(path.c_str(), O_RDONLY, (S_IRUSR | S_IRGRP | S_IROTH));
	if (fd == -1)
	{
	trace::warning(_X("Failed to map file. open(%s) failed with error %d"), path.c_str(), errno);
	return nullptr;
	}
	struct stat buf;
	if (fstat(fd, &buf) != 0)
	{
	trace::warning(_X("Failed to map file. fstat(%s) failed with error %d"), path.c_str(), errno);
	close(fd);
	return nullptr;
	}
	length = buf.st_size;
	void* address = mmap(nullptr, length, PROT_READ, MAP_SHARED, fd, 0);
	if(address == nullptr)
	{
	trace::warning(_X("Failed to map file. mmap(%s) failed with error %d"), path.c_str(), errno);
	close(fd);
	return nullptr;
	}
	close(fd);
	return address;
	}

And for Windows:

runtime/src/installer/corehost/common/pal.windows.cpp

Lines 79 to 117 in 1d5123f

	void* pal::map_file_readonly(const pal::string_t& path, size_t &length)
	{
	HANDLE file = CreateFileW(path.c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
	if (file == INVALID_HANDLE_VALUE)
	{
	trace::warning(_X("Failed to map file. CreateFileW(%s) failed with error %d"), path.c_str(), GetLastError());
	return nullptr;
	}
	LARGE_INTEGER fileSize;
	if (GetFileSizeEx(file, &fileSize) == 0)
	{
	trace::warning(_X("Failed to map file. GetFileSizeEx(%s) failed with error %d"), path.c_str(), GetLastError());
	CloseHandle(file);
	return nullptr;
	}
	length = (size_t)fileSize.QuadPart;
	HANDLE map = CreateFileMappingW(file, NULL, PAGE_READONLY, 0, 0, NULL);
	if (map == NULL)
	{
	trace::warning(_X("Failed to map file. CreateFileMappingW(%s) failed with error %d"), path.c_str(), GetLastError());
	CloseHandle(file);
	return nullptr;
	}
	void *address = MapViewOfFile(map, FILE_MAP_READ, 0, 0, 0);
	if (map == NULL)
	{
	trace::warning(_X("Failed to map file. MapViewOfFile(%s) failed with error %d"), path.c_str(), GetLastError());
	CloseHandle(file);
	return nullptr;
	}
	return address;
	}

As you can see, the Unix version closes the file reader before returning on the last branch, while the Windows version doesn't. Now we know the cause of this inconsistent behavior.
Possible solution A: change the pal.windows::map_file_readonly function to close the file before returning on the last branch.
Possible solution B: change the runner_t::extract function to call unmap_host before returning on the first branch.
Hope it helps.

[swaroop-sridhar]

Thanks a lot for the detailed analysis @Logerfo. I'll fix the issue, Thanks.

[VesCodes]

Is there an ETA on when this fix will be merged in a public release? Sorry to bring it up, but I'm not familiar with the runtime's update flow compared to dotnet/core and can't seem to find any relevant information. It's a bit of an annoying blocker and seems like a minor change that can get pushed through.

[swaroop-sridhar]

I'll port the change to CoreCLR repo for patching to 3.1 this week. I expect it'll make into March or April servicing release.

[VesCodes]

Any update on this making it to release @swaroop-sridhar ? It's still a blocker for us for self-updating apps. It looks to be slated for the 3.1.4 servicing release?

[swaroop-sridhar]

This change will be in this month's 3.1.4 release. Thanks.