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

Windows initiative 2018 #4808

Open
djdv opened this Issue Mar 12, 2018 · 19 comments

Comments

Projects
None yet
4 participants
@djdv
Contributor

djdv commented Mar 12, 2018

This issue serves as an anchor to monitor the broader topic of go-ipfs on Windows.
Here I will be highlighting the current issues being focused on and the PR efforts to resolve them. Feel free to add anything to this topic that relates to go-ipfs on Windows as a whole.

Build problems

  • make
    Fails to detect gx and gx-go properly
    #4510
    Resolved in: #4682

  • gx
    Fails to detect subtools properly (gx installs deps to wrong directory)
    whyrusleeping/gx#153
    Resolved in: whyrusleeping/gx#154

  • Build documentation
    A lot of people seem to have trouble building on Windows with the current document. I've proposed a revision here: #4691

interface problems

Windows Tests & CI
We need to pass tests on Windows so new issues can be caught by the CI before becoming a problem.
When Windows is skipped for testing, we suffer from regressions such as:
#4750
#4512
#3676

In addition to fixing existing tests, new tests should be made for testing platform specific behaviors.
Please comment about platform specific edge cases that should be considered.
For example, considering UNC paths (#1933 (comment))
Long prefix paths (https://github.com/ipfs/go-ipfs/pull/3328/files#diff-7ba0a8b1e26b8ed185c4824d4ee84164R503)
How :. behaves (#3328)
File system restrictions (#2013)
etc.

Feature parity with other platforms
The inability to pipe data, get certain data, mount IPFS, have readable output, etc. only pertain to Windows, platform native solutions for these features will have to be thought about, discussed, and implemented. The future issues and PRs will back-reference this issue so that development and discussion can be easily followed.

  • mount is not implemented
    currently not tracked
  • Pubsub is not reliable
    #4778

Everything else
There are many outstanding and outdated issues
https://github.com/ipfs/go-ipfs/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+amd64%2Fwindows+
They need to be reviewed and tagged
https://github.com/ipfs/go-ipfs/issues?q=is%3Aissue+is%3Aopen+label%3Awindows
Ones that are no longer relevant (already fixed) need to be marked as closed.

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 13, 2018

Pipes and stdin seem to be partially working, it's fine if the daemon is running but gets caught up on the lockfile otherwise, needs investigating, tests, etc.

partial pipe

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 13, 2018

Back-referencing other Windows related patches that are helpful to look at.

Building on Windows documentation [wip]
#4691

Building with make now works reliably (this should be in 0.4.14+)
#4682

gx correctly handles subtools on Windows (will be in the next release of gx)
whyrusleeping/gx#154.
For now, if you depend on this, you should build and install gx from the master branch, set IPFS_GX_USE_GLOBAL in your environment, and proceed to build go-ipfs using the latest version of gx and gx-go.

Windows was returning an ambiguous "Access is denied" when trying to fetch gx deps
whyrusleeping/gx#163

@Stebalien

This comment has been minimized.

Contributor

Stebalien commented Mar 13, 2018

Pipes and stdin seem to be partially working, it's fine if the daemon is running but gets caught up on the lockfile otherwise, needs investigating, tests, etc.

So, on Linux, the lockfile is automatically released when the process closes (by the OS). Unfortunately, windows appears to use the fallback which needs to be manually released. We may not be doing that somewhere.

Personally, I'd add some Windows specific logic to the lock package (https://github.com/camlistore/go4/tree/master/lock). Make it "succeed" if it can open the lock file with exclusive access, regardless of whether or not it already existed. Even better, have it mark the lock file for deletion on close.

@Kubuxu

This comment has been minimized.

Member

Kubuxu commented Mar 13, 2018

As @Stebalien said, it is same on Linux. The root of the issue is that shells start all processes in the pipe at the same time, no matter the pipe order so ipfs commands compete for repo lock.

@Stebalien

This comment has been minimized.

Contributor

Stebalien commented Mar 14, 2018

Personally, I'd like to fix this by starting the daemon in the background in a special mode that:

  1. Disables the DHT server.
  2. Shuts down after some amount of idle time.
  3. Has a really low connection limit?
@djdv

This comment has been minimized.

Contributor

djdv commented Mar 14, 2018

I've modified the go4 lock library, adding Windows calls to flag the lock for deletion by the OS when all handles are closed, and to not fail immediately when a lock file exists, but I'm having trouble blocking on successive lock attempts after that.
I'm using the LockFileEx systemcall with flags LOCKFILE_EXCLUSIVE_LOCK and without LOCKFILE_FAIL_IMMEDIATELY, which should put additional lock calls in a blocked state.
However I'm running into issues with that right now. Currently calls to lock just succeed 100% of the time instead of blocking, I'm unsure why at the moment.

@Stebalien

This comment has been minimized.

Contributor

Stebalien commented Mar 14, 2018

@djdv and I chatted over IRC. We've agreed to:

  1. Match what happens on linux and not block, just fail. However, @djdv's still going to fix the lock library to automatically clean up the lock file on close.
  2. Handle the pipe issue separately.
@djdv

This comment has been minimized.

Contributor

djdv commented Mar 14, 2018

Lock reference: go4org/go4#31

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 15, 2018

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 21, 2018

Making an effort to deal with file system restrictions:
#4850

@Stebalien

This comment has been minimized.

Contributor

Stebalien commented Mar 22, 2018

@djdv we should also look into #3971. Users hit this all the time. We may need to set some kind of "no index" option on the datastore.

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 23, 2018

I'm going to collect information on #3971 here.
While this doesn't work 100% of the time this is a good way to instigate the problem: #3971 (comment)
I've been running repo gc and doing that to instigate this. I'm using a near full 5900RPM HDD for testing, people seem to not have issues on faster drives.

In addition sometimes this happens: #4527
I set my root to G:\tmp\.ipfs and got a lot of garbage in G:\tmp\ which is my working directory. This is bad because something is feeding some call with garbage path names. The garbage content looks like valid block data just with mangled names.
Memory corruption is suspected to be responsible for this, but it's not certain as the filepaths reported via errors are correct, and not mangled.

14:52:13.053 ERROR flatfs: Err in Get() PathError err:&os.PathError{Op:"open", Path:"G:\tmp\.ipfs\blocks\AN\CIQFG5PESHPZCNK2T6BEKAT4AXP7TFIHDHQ36NKX6YXKCHYM5J6VANY.data", Err:0x20} -> open G:\tmp.ipfs\blocks\AN\CIQFG5PESHPZCNK2T6BEKAT4AXP7TFIHDHQ36NKX6YXKCHYM5J6VANY.data: The process cannot access the file because it is being used by another process.

Instigating this while running ProcMon is causing ProcMon to crash for me which isn't helping.
Running a separate tool I noticed several instances of buffer overflows being reported but I'm unsure as to where they originate in the Go source. In addition to this there are over-reads(STATUS_END_OF_FILE) for NtReadFile and the expected STATUS_ACCESS_DENIED for NtSetInformationFile which looks like it's trying to rename a file.
untitled

Finding out why and where filenames are getting corrupted seems like the current goal as it may be the root cause of this.

@Stebalien

This comment has been minimized.

Contributor

Stebalien commented Mar 23, 2018

I believe this was fixed (accidentally) by ipfs/go-ds-flatfs#34. go-os-rename wasn't properly keeping a temporary UTF16 buffers alive here: https://github.com/jbenet/go-os-rename/blob/3ac97f61ef67a6b87b95c1282f6c317ed0e693c2/rename_windows.go#L21-L40

Basically, it was taking the src and dst paths, converting them to UTF16 paths, and then converting those to uintptrs. However, this is only safe in two cases:

  1. If the the uintptr is constructed directly as an argument to the call to Syscall due to super-special go magic.
  2. If the buffers were marked as live (using runtime.KeepAlive) after it's last use.

Othewise, go will consider these buffers "free" after these lines:

https://github.com/jbenet/go-os-rename/blob/3ac97f61ef67a6b87b95c1282f6c317ed0e693c2/rename_windows.go#L31-L32

Congrats @djdv, you managed to fix a bug before looking into it (probably)!

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 23, 2018

Woohoo. I knew that lib was bad news. I was testing go-ipfs with updated gx deps and not encountering the issue on that branch master...djdv:big-deps but trying to find out which lib (out of at least 10) and which revision (some were multiple major revisions ahead) out of the bunch solved the issue, eluded me.
Thanks for investigating @Stebalien that issue has been a big problem for a while.

@Stebalien Stebalien referenced this issue Mar 24, 2018

Open

Audit Unsafe #4485

7 of 13 tasks complete
@djdv

This comment has been minimized.

Contributor

djdv commented Mar 25, 2018

An inconsistency in Windows specific behaviour in/on Go is being discussed here: golang/go#24482
This is related to an inconsistency where ipfs get *somehash* -o NUL works but ipfs get *somehash* -o nul will fail with a path error.
This will influence whyrusleeping/tar-utils#2

@djdv

This comment has been minimized.

Contributor

djdv commented Mar 27, 2018

The previous Go patch has resolved the case-sensitive NUL issue for files, I've created a separate issue here: golang/go#24556
which prevents us from directing get output to NUL when the hash is a directory (not a file)
i.e.
ipfs get QmQine7gvHncNevKtG9QXxf3nXcwSj6aDDmMm52mHofEEp -o NUL
Should succeed, but currently(Go 1.10) does not.

@djdv

This comment has been minimized.

Contributor

djdv commented Apr 6, 2018

Creation of symlinks is proving to be a problem in go-ipfs and gx
#4906
https://discuss.ipfs.io/t/issus-build-from-source/2445

Decisions are going to have to be made in whyrusleeping/tar-utils#2 on how best to handle this.

On Windows, users can't create symlinks without special privileges (see section below if interested in details), but we don't want to require users to have these privileges for normal operations.

Since symlinks are restricted for normal users on Windows, the current plan is to simply warn the user and skip symlinks if we don't have permission to create them, but still extract everything else as normal.
If we do have permission, there's no warning and we create symlinks as normal.

Requiring symlink privileges instead of opting to skip symlinks, prevents unprivileged users from being able to get any hash with a symlink in it, regardless of if it's desired or not. This currently means we won't be able to build go-ipfs without special rights on Windows as some of our dependencies utilize symlinks in their tests.


Starting with Windows 10-1703(14972), users can enable "Developer Mode" in their system settings, this allows normal users to create symlinks in Windows 10-aware applications, without elevated process rights.
There's currently a pull request in Go to enable support for this golang/go#24307 so it will likely be enabled for all Go programs in Go 1.11.
So after Go 1.11, go-ipfs Windows developers can simply enable Dev-mode and not have to worry about anything.

Prior to Go 1.11 and for users below Windows version 10-1703, users will need to hold the SeCreateSymbolicLinkPrivilege right in their access token.
The easiest way to attain this is by simply running the process as admin, which should hold this right.
To create them as a normal user in an unprivileged process, the process is more complex than simply adding the privilege to your account because of the way UAC works (see below).
Depending on how things go, a user friendly method of enabling unprivileged symlink creation may have to be written up for Windows <10 users who want to create symlinks without requiring full admin rights.

More information about symlinks on Windows can be found here
Unprivileged creation in W10:
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
About SeCreateSymbolicLinkPrivilege:
https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links
Why can't I just add SeCreateSymbolicLinkPrivilege to my account?
https://msdn.microsoft.com/en-us/library/bb530410.aspx#vistauac_topic3 [see subsection: Access Token Changes]

@djdv

This comment has been minimized.

Contributor

djdv commented Apr 19, 2018

Symlink creation update
symlinks

On Vista+, as long as the user has the privilege to create links, they should now be able to.
If they don't have that privilege, as a compromise, links will be skipped and a warning will be logged instead of failing out.

How the warning will look isn't finalized yet, it may be shorter or contain a link to documentation telling users how to enable symlinks if they need them.

Unprivileged link creation via Windows 10's Developer Mode, still relies on golang/go#24307, if you have this patch, link creation should work as expected.

Edit: PR is up
#4956

@djdv

This comment has been minimized.

Contributor

djdv commented May 7, 2018

ipfs mount discussion will take place here: #5003

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment