Hacker News Reader

Comment by masklinn | Hacker News Reader

Impressive. It would be difficult for you to be more wrong, and reading the article and noting that

> [ex-coworkers at microsoft] confirmed that my fix should mitigate the bug (I’d already noted that it had allowed ~600 clean builds in a row), and promised to create a proper fix in Windows.

would have saved you the bother of nerd-sniping.

The coherency notes in the article you link are irrelevant because they're about concurrent access. In TFA, the file is created (through mapping), closed, then executed. Sequentially. Also no ReadFile or WriteFile involved (in fact though I don't know how Windows implements it one would expect PE loading to use memory mappings, and thus be covered under coherence guarantees)

View on HN

Sure, you can call your "ex-coworkers at Microsoft" (most of which are irrelevant to debugging/verifying this issue) - or, you can just spend some time reading the documentation, as previously suggested.

The coherency issues are your "tread lightly" warning. The docs regarding FlushViewOfFile expand on this specific issue:

Flushing a range of a mapped view initiates writing of dirty pages within that range to the disk. Dirty pages are those whose contents have changed since the file view was mapped. The FlushViewOfFile function does not flush the file metadata, and it does not wait to return until the changes are flushed from the underlying hardware disk cache and physically written to disk. To flush all the dirty pages plus the metadata for the file and ensure that they are physically written to disk, call FlushViewOfFile and then call the FlushFileBuffers function.

So, after carefully reading this documentation, we can clearly conclude OP's pull-request is actually missing a call to FlushViewOfFile prior to calling FlushFileBuffers.

This is incorrect. FlushFiewOfFile and FlushFileBuffers only guarantees that contents are written to the physical media. It is not necessary (well, it is not supposed to be necessary) to call either of these functions for other processes to be able to see the changes.

The whole point of a cache is that if A writes something, and then B reads it after A finishes writing it, B should see the results of A's modification. Whether it has been flushed to the physical media is irrelevant, because if it hasn't the cache manager can still serve the contents directly from the cache. And a call to Flush is not necessary for that.

So, the OP's change is not missing anything. As the article explains, it is in fact the Windows kernel that is behaving incorrectly (which was also confirmed by people who work on the windows kernel).

Closing the file is supposed to ensure coherency. It generally does. That it occasionally does not is a bug, that Microsoft will fix.

Reply on news.ycombinator.com