Win32: use a more modern API call in FileCommit() #3450

• this seems to be a more recent API call and also supports e.g. SMB3, ReFS, which is not guaranteed for commit_()

See http://msdn.microsoft.com/en-us/library/windows/desktop/aa364439%28v=vs.85%29.aspx

Add the MSDN link to the git commit message.

@jgarzik Done...

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/4c0b2cde3a68e65971ab7d9970a3419328d4fe0e for binaries and test log.

This could happen for one of several reasons:

1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
4. The test suite fails on either Linux i386 or Win32
5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

Failuer because of:

<pre> tail: write error: Broken pipe tail: write error </pre>

AFAIK unrelated to this pull, as I only changed the commit-msg and @BitcoinPullTester was happy with the code before ;).

This needs to be tested really, really well.

Judging from the lack of MacOSX-esque corruption on windows the current _commit call is working. Switching this around may lead to some very hard to debug issues. Experience with the MacOSX issue has shown that API documentation can be misleading or even plainly wrong.

This is code used for the block files, not LevelDB (which is where the OSX problem was). Even if _commit didn't work well, we'd expect a different failure pattern.

Regardless, this indeed needs serious testing. On Dec 21, 2013 10:37 AM, "Wladimir J. van der Laan" < notifications@github.com> wrote:

This needs to be tested really, really well.

Judging from the lack of MacOSX-esque corruption on windows the current _commit call is working. Switching this around may lead to some very hard to debug issues. Experience with the MacOSX issue has shown that API documentation can be misleading or even plainly wrong.

— Reply to this email directly or view it on GitHubhttps://github.com/bitcoin/bitcoin/pull/3450#issuecomment-31060070 .

To prevent corruption issues the data sync call for MacOSX was also changed (see e7bad10). In any case you raise a good point: what does leveldb use on Windows. This is the Sync function for win32: https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/util/env_win.cc#L587

It calls FlushFileBuffers (for unmapped regions) and FlushViewOfFile (for mappings).

Given that there is no LevelDB corruption issue on Windows, this change is probably safe.

@laanwj Thanks for digging into the LevelDB code, to verify they also use this API call.

@laanwj Good to know.

Any objections to merge this or how I can help with this? I'm running with this patch for months.

I don't have objections... I think it makes sense to do this consistent with leveldb.

ACK

ACK