From d1d2adb5dc04edbbffdd851154835cb8dc8bb6ee Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Fri, 29 Oct 2021 18:19:49 +0000 Subject: [PATCH] Fix a bug where setAccessTime would also set the modified time on Windows. TEST=issue_35112_test.dart Bug: https://github.com/dart-lang/sdk/issues/35112 Change-Id: I4c7a1795250447888ec1cbde71bf2799c3467b55 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/218182 Commit-Queue: Brian Quinlan Reviewed-by: Alexander Aprelev --- runtime/bin/file_win.cc | 25 +++++++++---- runtime/bin/utils_win.cc | 37 ++++++++++++------- runtime/bin/utils_win.h | 3 ++ tests/standalone/io/issue_35112_test.dart | 37 +++++++++++++++++++ tests/standalone_2/io/issue_35112_test.dart | 39 +++++++++++++++++++++ 5 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 tests/standalone/io/issue_35112_test.dart create mode 100644 tests/standalone_2/io/issue_35112_test.dart diff --git a/runtime/bin/file_win.cc b/runtime/bin/file_win.cc index b7f5dbee115..bb24d000733 100644 --- a/runtime/bin/file_win.cc +++ b/runtime/bin/file_win.cc @@ -935,18 +935,29 @@ time_t File::LastModified(Namespace* namespc, const char* name) { bool File::SetLastAccessed(Namespace* namespc, const char* name, int64_t millis) { - // First get the current times. struct __stat64 st; Utf8ToWideScope system_name(PrefixLongFilePath(name)); - if (!StatHelper(system_name.wide(), &st)) { + if (!StatHelper(system_name.wide(), &st)) { // Checks that it is a file. return false; } - // Set the new time: - struct __utimbuf64 times; - times.actime = millis / kMillisecondsPerSecond; - times.modtime = st.st_mtime; - return _wutime64(system_name.wide(), ×) == 0; + // _utime and related functions set the access and modification times of the + // affected file. Even if the specified modification time is not changed + // from the current value, _utime will trigger a file modification event + // (e.g. ReadDirectoryChangesW will report the file as modified). + // + // So set the file access time directly using SetFileTime. + FILETIME at = GetFiletimeFromMillis(millis); + HANDLE file_handle = + CreateFileW(system_name.wide(), FILE_WRITE_ATTRIBUTES, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr); + if (file_handle == INVALID_HANDLE_VALUE) { + return false; + } + bool result = SetFileTime(file_handle, nullptr, &at, nullptr); + CloseHandle(file_handle); + return result; } bool File::SetLastModified(Namespace* namespc, diff --git a/runtime/bin/utils_win.cc b/runtime/bin/utils_win.cc index fcbbcbbe3c6..174ed7fe37f 100644 --- a/runtime/bin/utils_win.cc +++ b/runtime/bin/utils_win.cc @@ -16,6 +16,23 @@ namespace dart { namespace bin { +// The offset between a `FILETIME` epoch (January 1, 1601 UTC) and a Unix +// epoch (January 1, 1970 UTC) measured in 100ns intervals. +// +// See https://docs.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-filetime +static const int64_t kFileTimeEpoch = 116444736000000000LL; + +// Although win32 uses 64-bit integers for representing timestamps, +// these are packed into a FILETIME structure. The FILETIME +// structure is just a struct representing a 64-bit integer. The +// TimeStamp union allows access to both a FILETIME and an integer +// representation of the timestamp. The Windows timestamp is in +// 100-nanosecond intervals since January 1, 1601. +union TimeStamp { + FILETIME ft_; + int64_t t_; +}; + void FormatMessageIntoBuffer(DWORD code, wchar_t* buffer, int buffer_length) { DWORD message_size = FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, NULL, code, @@ -31,6 +48,12 @@ void FormatMessageIntoBuffer(DWORD code, wchar_t* buffer, int buffer_length) { buffer[buffer_length - 1] = 0; } +FILETIME GetFiletimeFromMillis(int64_t millis) { + static const int64_t kTimeScaler = 10000; // 100 ns to ms. + TimeStamp t = {.t_ = millis * kTimeScaler + kFileTimeEpoch}; + return t.ft_; +} + OSError::OSError() : sub_system_(kSystem), code_(0), message_(NULL) { Reload(); } @@ -165,24 +188,12 @@ bool ShellUtils::GetUtf8Argv(int argc, char** argv) { return true; } -// Although win32 uses 64-bit integers for representing timestamps, -// these are packed into a FILETIME structure. The FILETIME -// structure is just a struct representing a 64-bit integer. The -// TimeStamp union allows access to both a FILETIME and an integer -// representation of the timestamp. The Windows timestamp is in -// 100-nanosecond intervals since January 1, 1601. -union TimeStamp { - FILETIME ft_; - int64_t t_; -}; - static int64_t GetCurrentTimeMicros() { - static const int64_t kTimeEpoc = 116444736000000000LL; static const int64_t kTimeScaler = 10; // 100 ns to us. TimeStamp time; GetSystemTimeAsFileTime(&time.ft_); - return (time.t_ - kTimeEpoc) / kTimeScaler; + return (time.t_ - kFileTimeEpoch) / kTimeScaler; } static int64_t qpc_ticks_per_second = 0; diff --git a/runtime/bin/utils_win.h b/runtime/bin/utils_win.h index 3d4cfe389c0..facee434aad 100644 --- a/runtime/bin/utils_win.h +++ b/runtime/bin/utils_win.h @@ -18,6 +18,9 @@ namespace bin { void FormatMessageIntoBuffer(DWORD code, wchar_t* buffer, int buffer_length); +// Convert from milliseconds since the Unix epoch to a FILETIME. +FILETIME GetFiletimeFromMillis(int64_t millis); + // These string utility functions return strings that have been allocated with // Dart_ScopeAllocate(). They should be used only when we are inside an API // scope. If a string returned by one of these functions must persist beyond diff --git a/tests/standalone/io/issue_35112_test.dart b/tests/standalone/io/issue_35112_test.dart new file mode 100644 index 00000000000..1d53f1e0194 --- /dev/null +++ b/tests/standalone/io/issue_35112_test.dart @@ -0,0 +1,37 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'dart:io'; + +import 'package:expect/expect.dart'; +import 'test_utils.dart' show withTempDir; + +main() async { + // Verify that File.setLastAccessed does *not* trigger a FileSystemModifyEvent + // with FileSystemModifyEvent.contentChanged == true. + await withTempDir('issue_35112', (Directory tempDir) async { + File file = new File("${tempDir.path}/file.tmp"); + file.createSync(); + + final eventCompleter = new Completer(); + StreamSubscription? subscription; + subscription = tempDir.watch().listen((FileSystemEvent event) { + if (event is FileSystemModifyEvent && event.contentChanged) { + eventCompleter.complete(event); + } + subscription?.cancel(); + }); + + file.setLastAccessedSync(DateTime.now().add(Duration(days: 3))); + Timer(Duration(seconds: 1), () { + eventCompleter.complete(null); + subscription?.cancel(); + }); + ; + FileSystemEvent? event = await eventCompleter.future; + Expect.isNull(event, + "No event should be triggered or .contentChanged should equal false"); + }); +} diff --git a/tests/standalone_2/io/issue_35112_test.dart b/tests/standalone_2/io/issue_35112_test.dart new file mode 100644 index 00000000000..1c3cfd31bc9 --- /dev/null +++ b/tests/standalone_2/io/issue_35112_test.dart @@ -0,0 +1,39 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// @dart = 2.9 + +import 'dart:async'; +import 'dart:io'; + +import 'package:expect/expect.dart'; +import 'test_utils.dart' show withTempDir; + +main() async { + // Verify that File.setLastAccessed does *not* trigger a FileSystemModifyEvent + // with FileSystemModifyEvent.contentChanged == true. + await withTempDir('issue_35112', (Directory tempDir) async { + File file = new File("${tempDir.path}/file.tmp"); + file.createSync(); + + final eventCompleter = new Completer(); + StreamSubscription subscription; + subscription = tempDir.watch().listen((FileSystemEvent event) { + if (event is FileSystemModifyEvent && event.contentChanged) { + eventCompleter.complete(event); + } + subscription?.cancel(); + }); + + file.setLastAccessedSync(DateTime.now().add(Duration(days: 3))); + Timer(Duration(seconds: 1), () { + eventCompleter.complete(null); + subscription?.cancel(); + }); + ; + FileSystemEvent event = await eventCompleter.future; + Expect.isNull(event, + "No event should be triggered or .contentChanged should equal false"); + }); +}