[res] Better modification time resolution in Idmap
We used to track the modification time in seconds, which is both
imprecise (an apk installation + idmap generation can easily
take less time) and forces us to wait for >1s in the tests to
just check if up-to-date checks work.
This change updates the time to nanosecond resolution where
supported (hm, MinGW for Windows, hm), as the underlying
OS API provides
Test: build + atest libandroidfw_tests idmap2_tests + boot
Flag: EXEMPT minor change
Change-Id: I37635e6f3b62aff3b4794912ac585a9ef5ea7a1e
diff --git a/libs/androidfw/AssetManager.cpp b/libs/androidfw/AssetManager.cpp
index e618245..5955915 100644
--- a/libs/androidfw/AssetManager.cpp
+++ b/libs/androidfw/AssetManager.cpp
@@ -1420,18 +1420,20 @@
Mutex AssetManager::SharedZip::gLock;
DefaultKeyedVector<String8, wp<AssetManager::SharedZip> > AssetManager::SharedZip::gOpen;
-AssetManager::SharedZip::SharedZip(const String8& path, time_t modWhen)
- : mPath(path), mZipFile(NULL), mModWhen(modWhen),
- mResourceTableAsset(NULL), mResourceTable(NULL)
-{
- if (kIsDebug) {
- ALOGI("Creating SharedZip %p %s\n", this, mPath.c_str());
- }
- ALOGV("+++ opening zip '%s'\n", mPath.c_str());
- mZipFile = ZipFileRO::open(mPath.c_str());
- if (mZipFile == NULL) {
- ALOGD("failed to open Zip archive '%s'\n", mPath.c_str());
- }
+AssetManager::SharedZip::SharedZip(const String8& path, ModDate modWhen)
+ : mPath(path),
+ mZipFile(NULL),
+ mModWhen(modWhen),
+ mResourceTableAsset(NULL),
+ mResourceTable(NULL) {
+ if (kIsDebug) {
+ ALOGI("Creating SharedZip %p %s\n", this, mPath.c_str());
+ }
+ ALOGV("+++ opening zip '%s'\n", mPath.c_str());
+ mZipFile = ZipFileRO::open(mPath.c_str());
+ if (mZipFile == NULL) {
+ ALOGD("failed to open Zip archive '%s'\n", mPath.c_str());
+ }
}
AssetManager::SharedZip::SharedZip(int fd, const String8& path)
@@ -1453,7 +1455,7 @@
bool createIfNotPresent)
{
AutoMutex _l(gLock);
- time_t modWhen = getFileModDate(path.c_str());
+ auto modWhen = getFileModDate(path.c_str());
sp<SharedZip> zip = gOpen.valueFor(path).promote();
if (zip != NULL && zip->mModWhen == modWhen) {
return zip;
@@ -1520,8 +1522,8 @@
bool AssetManager::SharedZip::isUpToDate()
{
- time_t modWhen = getFileModDate(mPath.c_str());
- return mModWhen == modWhen;
+ auto modWhen = getFileModDate(mPath.c_str());
+ return mModWhen == modWhen;
}
void AssetManager::SharedZip::addOverlay(const asset_path& ap)
diff --git a/libs/androidfw/include/androidfw/AssetManager.h b/libs/androidfw/include/androidfw/AssetManager.h
index ce0985b..376c881 100644
--- a/libs/androidfw/include/androidfw/AssetManager.h
+++ b/libs/androidfw/include/androidfw/AssetManager.h
@@ -280,21 +280,21 @@
~SharedZip();
private:
- SharedZip(const String8& path, time_t modWhen);
- SharedZip(int fd, const String8& path);
- SharedZip(); // <-- not implemented
+ SharedZip(const String8& path, ModDate modWhen);
+ SharedZip(int fd, const String8& path);
+ SharedZip(); // <-- not implemented
- String8 mPath;
- ZipFileRO* mZipFile;
- time_t mModWhen;
+ String8 mPath;
+ ZipFileRO* mZipFile;
+ ModDate mModWhen;
- Asset* mResourceTableAsset;
- ResTable* mResourceTable;
+ Asset* mResourceTableAsset;
+ ResTable* mResourceTable;
- Vector<asset_path> mOverlays;
+ Vector<asset_path> mOverlays;
- static Mutex gLock;
- static DefaultKeyedVector<String8, wp<SharedZip> > gOpen;
+ static Mutex gLock;
+ static DefaultKeyedVector<String8, wp<SharedZip> > gOpen;
};
/*
diff --git a/libs/androidfw/include/androidfw/Idmap.h b/libs/androidfw/include/androidfw/Idmap.h
index 64b1f0c..98f1aa8 100644
--- a/libs/androidfw/include/androidfw/Idmap.h
+++ b/libs/androidfw/include/androidfw/Idmap.h
@@ -25,8 +25,9 @@
#include "android-base/macros.h"
#include "android-base/unique_fd.h"
#include "androidfw/ConfigDescription.h"
-#include "androidfw/StringPiece.h"
#include "androidfw/ResourceTypes.h"
+#include "androidfw/StringPiece.h"
+#include "androidfw/misc.h"
#include "utils/ByteOrder.h"
namespace android {
@@ -202,7 +203,7 @@
android::base::unique_fd idmap_fd_;
std::string_view overlay_apk_path_;
std::string_view target_apk_path_;
- time_t idmap_last_mod_time_;
+ ModDate idmap_last_mod_time_;
private:
DISALLOW_COPY_AND_ASSIGN(LoadedIdmap);
diff --git a/libs/androidfw/include/androidfw/misc.h b/libs/androidfw/include/androidfw/misc.h
index 077609d..09ae40c 100644
--- a/libs/androidfw/include/androidfw/misc.h
+++ b/libs/androidfw/include/androidfw/misc.h
@@ -13,14 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+#pragma once
-#include <sys/types.h>
+#include <time.h>
//
// Handy utility functions and portability code.
//
-#ifndef _LIBS_ANDROID_FW_MISC_H
-#define _LIBS_ANDROID_FW_MISC_H
namespace android {
@@ -41,15 +40,35 @@
} FileType;
/* get the file's type; follows symlinks */
FileType getFileType(const char* fileName);
-/* get the file's modification date; returns -1 w/errno set on failure */
-time_t getFileModDate(const char* fileName);
+
+// MinGW doesn't support nanosecond resolution in stat() modification time, and given
+// that it only matters on the device it's ok to keep it at the second level there.
+#ifdef _WIN32
+using ModDate = time_t;
+inline constexpr ModDate kInvalidModDate = ModDate(-1);
+inline constexpr unsigned long long kModDateResolutionNs = 1ull * 1000 * 1000 * 1000;
+inline time_t toTimeT(ModDate m) {
+ return m;
+}
+#else
+using ModDate = timespec;
+inline constexpr ModDate kInvalidModDate = {-1, -1};
+inline constexpr unsigned long long kModDateResolutionNs = 1;
+inline time_t toTimeT(ModDate m) {
+ return m.tv_sec;
+}
+#endif
+
+/* get the file's modification date; returns kInvalidModDate w/errno set on failure */
+ModDate getFileModDate(const char* fileName);
/* same, but also returns -1 if the file has already been deleted */
-time_t getFileModDate(int fd);
+ModDate getFileModDate(int fd);
// Check if |path| or |fd| resides on a readonly filesystem.
bool isReadonlyFilesystem(const char* path);
bool isReadonlyFilesystem(int fd);
-}; // namespace android
+} // namespace android
-#endif // _LIBS_ANDROID_FW_MISC_H
+// Whoever uses getFileModDate() will need this as well
+bool operator==(const timespec& l, const timespec& r);
diff --git a/libs/androidfw/misc.cpp b/libs/androidfw/misc.cpp
index 93dcaf5..9bdaf18a 100644
--- a/libs/androidfw/misc.cpp
+++ b/libs/androidfw/misc.cpp
@@ -28,11 +28,13 @@
#include <sys/vfs.h>
#endif // __linux__
-#include <cstring>
-#include <cstdio>
#include <errno.h>
#include <sys/stat.h>
+#include <cstdio>
+#include <cstring>
+#include <tuple>
+
namespace android {
/*
@@ -73,27 +75,32 @@
}
}
-/*
- * Get a file's modification date.
- */
-time_t getFileModDate(const char* fileName) {
- struct stat sb;
- if (stat(fileName, &sb) < 0) {
- return (time_t)-1;
- }
- return sb.st_mtime;
+static ModDate getModDate(const struct stat& st) {
+#ifdef _WIN32
+ return st.st_mtime;
+#else
+ return st.st_mtim;
+#endif
}
-time_t getFileModDate(int fd) {
- struct stat sb;
- if (fstat(fd, &sb) < 0) {
- return (time_t)-1;
- }
- if (sb.st_nlink <= 0) {
- errno = ENOENT;
- return (time_t)-1;
- }
- return sb.st_mtime;
+ModDate getFileModDate(const char* fileName) {
+ struct stat sb;
+ if (stat(fileName, &sb) < 0) {
+ return kInvalidModDate;
+ }
+ return getModDate(sb);
+}
+
+ModDate getFileModDate(int fd) {
+ struct stat sb;
+ if (fstat(fd, &sb) < 0) {
+ return kInvalidModDate;
+ }
+ if (sb.st_nlink <= 0) {
+ errno = ENOENT;
+ return kInvalidModDate;
+ }
+ return getModDate(sb);
}
#ifndef __linux__
@@ -124,4 +131,8 @@
}
#endif // __linux__
-}; // namespace android
+} // namespace android
+
+bool operator==(const timespec& l, const timespec& r) {
+ return std::tie(l.tv_sec, l.tv_nsec) == std::tie(r.tv_sec, l.tv_nsec);
+}
diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp
index 60aa7d8..cb2e56f 100644
--- a/libs/androidfw/tests/Idmap_test.cpp
+++ b/libs/androidfw/tests/Idmap_test.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#include <chrono>
+#include <thread>
+
#include "android-base/file.h"
#include "androidfw/ApkAssets.h"
#include "androidfw/AssetManager2.h"
@@ -27,6 +30,7 @@
#include "data/overlayable/R.h"
#include "data/system/R.h"
+using namespace std::chrono_literals;
using ::testing::NotNull;
namespace overlay = com::android::overlay;
@@ -218,10 +222,13 @@
unlink(temp_file.path);
ASSERT_FALSE(apk_assets->IsUpToDate());
- sleep(2);
+
+ const auto sleep_duration =
+ std::chrono::nanoseconds(std::max(kModDateResolutionNs, 1'000'000ull));
+ std::this_thread::sleep_for(sleep_duration);
base::WriteStringToFile("hello", temp_file.path);
- sleep(2);
+ std::this_thread::sleep_for(sleep_duration);
ASSERT_FALSE(apk_assets->IsUpToDate());
}
diff --git a/tools/aapt/Package.cpp b/tools/aapt/Package.cpp
index 5e0f87f..60c4bf5 100644
--- a/tools/aapt/Package.cpp
+++ b/tools/aapt/Package.cpp
@@ -292,13 +292,12 @@
}
if (!hasData) {
const String8& srcName = file->getSourceFile();
- time_t fileModWhen;
- fileModWhen = getFileModDate(srcName.c_str());
- if (fileModWhen == (time_t) -1) { // file existence tested earlier,
- return false; // not expecting an error here
+ auto fileModWhen = getFileModDate(srcName.c_str());
+ if (fileModWhen == kInvalidModDate) { // file existence tested earlier,
+ return false; // not expecting an error here
}
-
- if (fileModWhen > entry->getModWhen()) {
+
+ if (toTimeT(fileModWhen) > entry->getModWhen()) {
// mark as deleted so add() will succeed
if (bundle->getVerbose()) {
printf(" (removing old '%s')\n", storageName.c_str());