Introduce InputFile/OutputFile and FileMutex.
FileHandle is replaced with InputFile/OutputFile and FileMutex.
Use InputFile when you want to open a file in read-only.
USe OutputFile when you open a file for writing.
Both of them provide a reliable way to access the files and perform
the I/O operations.
Given a name "foo", FileMutex creates a file named "foo.lock" and
tries to acquire an advisory lock (flock) on this file.
FileHandle, which uses the file it's openning for locking, may corrupt
the file contents when two or more processes are trying to gain the
lock for reading/writing. For example:
Process #2 creates foo
Process #1 opens foo
Process #2 opens foo
Process #2 locks foo (exclusively) (success)
Process #1 locks foo (failed, retry #1)
Process #2 starts writing foo
Process #1 opens and truncates foo (note there’s O_TRUNC in the flag)
Process #2 writes foo continually (foo is corrupted from now on ...)
Process #1 locks foo (failed, retry #2)
...
Process #1 locks foo (reach the max retries and return)
Process #2 gets done on writing foo (foo is corrupted ...)
Process #2 unlocks and closes foo (foo is corrupted)
diff --git a/lib/ExecutionEngine/RSScript.cpp b/lib/ExecutionEngine/RSScript.cpp
index af37ab6..d223eee 100644
--- a/lib/ExecutionEngine/RSScript.cpp
+++ b/lib/ExecutionEngine/RSScript.cpp
@@ -35,8 +35,10 @@
#include "CompilerOption.h"
#include "DebugHelper.h"
-#include "FileHandle.h"
+#include "FileMutex.h"
#include "GDBJITRegistrar.h"
+#include "InputFile.h"
+#include "OutputFile.h"
#include "ScriptCompiled.h"
#include "ScriptCached.h"
#include "Sha1Helper.h"
@@ -99,7 +101,6 @@
return;
}
-
bool RSScript::doReset() {
resetState();
return true;
@@ -133,9 +134,10 @@
return status;
}
- FileHandle objFile;
- if (objFile.open(objPath, OpenMode::Write) < 1) {
- ALOGE("Failed to open %s for write.\n", objPath);
+ OutputFile objFile(objPath);
+ if (objFile.hasError()) {
+ ALOGE("Failed to open %s for write. (%s)", objPath,
+ objFile.getErrorMessage().c_str());
return 1;
}
@@ -218,15 +220,37 @@
std::string objPath = getCachedObjectPath();
std::string infoPath = getCacheInfoPath();
- FileHandle objFile;
- if (objFile.open(objPath.c_str(), OpenMode::Read) < 0) {
- // Unable to open the executable file in read mode.
+ // Locks for reading object file and info file.
+ FileMutex<FileBase::kReadLock> objFileMutex(objPath);
+ FileMutex<FileBase::kReadLock> infoFileMutex(infoPath);
+
+ // Aquire read lock for object file.
+ if (objFileMutex.hasError() || !objFileMutex.lock()) {
+ ALOGE("Unable to acquire the lock for %s! (%s)", objPath.c_str(),
+ objFileMutex.getErrorMessage().c_str());
return 1;
}
- FileHandle infoFile;
- if (infoFile.open(infoPath.c_str(), OpenMode::Read) < 0) {
- // Unable to open the metadata information file in read mode.
+ // Aquire read lock for info file.
+ if (infoFileMutex.hasError() || !infoFileMutex.lock()) {
+ ALOGE("Unable to acquire the lock for %s! (%s)", infoPath.c_str(),
+ infoFileMutex.getErrorMessage().c_str());
+ return 1;
+ }
+
+ // Open the object file and info file
+ InputFile objFile(objPath);
+ InputFile infoFile(infoPath);
+
+ if (objFile.hasError()) {
+ ALOGE("Unable to open %s for reading! (%s)", objPath.c_str(),
+ objFile.getErrorMessage().c_str());
+ return 1;
+ }
+
+ if (infoFile.hasError()) {
+ ALOGE("Unable to open %s for reading! (%s)", infoPath.c_str(),
+ infoFile.getErrorMessage().c_str());
return 1;
}
@@ -250,10 +274,10 @@
}
if (checkOnly)
- return !reader.checkCacheFile(&objFile, &infoFile, this);
+ return !reader.checkCacheFile(objFile, infoFile, this);
// Read cache file
- ScriptCached *cached = reader.readCacheFile(&objFile, &infoFile, this);
+ ScriptCached *cached = reader.readCacheFile(objFile, infoFile, this);
if (!cached) {
mIsContextSlotNotAvail = reader.isContextSlotNotAvail();
@@ -318,72 +342,92 @@
// we don't have to cache it.
if (isCacheable()) {
-
std::string objPath = getCachedObjectPath();
std::string infoPath = getCacheInfoPath();
- // Remove the file if it already exists before writing the new file.
- // The old file may still be mapped elsewhere in memory and we do not want
- // to modify its contents. (The same script may be running concurrently in
- // the same process or a different process!)
- ::unlink(objPath.c_str());
- ::unlink(infoPath.c_str());
+ // Locks for writing object file and info file.
+ FileMutex<FileBase::kWriteLock> objFileMutex(objPath);
+ FileMutex<FileBase::kWriteLock> infoFileMutex(infoPath);
- FileHandle objFile;
- FileHandle infoFile;
+ // Aquire write lock for object file.
+ if (objFileMutex.hasError() || !objFileMutex.lock()) {
+ ALOGE("Unable to acquire the lock for %s! (%s)", objPath.c_str(),
+ objFileMutex.getErrorMessage().c_str());
+ return 1;
+ }
- if (objFile.open(objPath.c_str(), OpenMode::Write) >= 0 &&
- infoFile.open(infoPath.c_str(), OpenMode::Write) >= 0) {
+ // Aquire write lock for info file.
+ if (infoFileMutex.hasError() || !infoFileMutex.lock()) {
+ ALOGE("Unable to acquire the lock for %s! (%s)", infoPath.c_str(),
+ infoFileMutex.getErrorMessage().c_str());
+ return 1;
+ }
- MCCacheWriter writer;
+ // Open the object file and info file
+ OutputFile objFile(objPath);
+ OutputFile infoFile(infoPath);
+
+ if (objFile.hasError()) {
+ ALOGE("Unable to open %s for writing! (%s)", objPath.c_str(),
+ objFile.getErrorMessage().c_str());
+ return 1;
+ }
+
+ if (infoFile.hasError()) {
+ ALOGE("Unable to open %s for writing! (%s)", infoPath.c_str(),
+ infoFile.getErrorMessage().c_str());
+ return 1;
+ }
+
+ MCCacheWriter writer;
#ifdef TARGET_BUILD
- // Dependencies
- writer.addDependency(BCC_FILE_RESOURCE, pathLibBCC_SHA1, sha1LibBCC_SHA1);
- writer.addDependency(BCC_FILE_RESOURCE, pathLibRS, sha1LibRS);
+ // Dependencies
+ writer.addDependency(BCC_FILE_RESOURCE, pathLibBCC_SHA1, sha1LibBCC_SHA1);
+ writer.addDependency(BCC_FILE_RESOURCE, pathLibRS, sha1LibRS);
#endif
- for (unsigned i = 0; i < mSourceDependencies.size(); i++) {
- const SourceDependency *source_dep = mSourceDependencies[i];
- writer.addDependency(source_dep->getSourceType(),
- source_dep->getSourceName(),
- source_dep->getSHA1Checksum());
+ for (unsigned i = 0; i < mSourceDependencies.size(); i++) {
+ const SourceDependency *source_dep = mSourceDependencies[i];
+ writer.addDependency(source_dep->getSourceType(),
+ source_dep->getSourceName(),
+ source_dep->getSHA1Checksum());
+ }
+
+
+ // libRS is threadable dirty hack
+ // TODO: This should be removed in the future
+ uint32_t libRS_threadable = 0;
+ if (mpExtSymbolLookupFn) {
+ libRS_threadable =
+ (uint32_t)mpExtSymbolLookupFn(mpExtSymbolLookupFnContext,
+ "__isThreadable");
+ }
+
+ if (!writer.writeCacheFile(objFile, infoFile, this, libRS_threadable)) {
+ // Erase the file contents.
+ objFile.truncate();
+ infoFile.truncate();
+
+ // Close the file such that we can removed it from the filesystem.
+ objFile.close();
+ infoFile.close();
+
+ if (::unlink(objPath.c_str()) != 0) {
+ ALOGE("Unable to remove the invalid cache file: %s. (reason: %s)\n",
+ objPath.c_str(), ::strerror(errno));
}
-
- // libRS is threadable dirty hack
- // TODO: This should be removed in the future
- uint32_t libRS_threadable = 0;
- if (mpExtSymbolLookupFn) {
- libRS_threadable =
- (uint32_t)mpExtSymbolLookupFn(mpExtSymbolLookupFnContext,
- "__isThreadable");
- }
-
- if (!writer.writeCacheFile(&objFile, &infoFile, this, libRS_threadable)) {
- objFile.truncate();
- objFile.close();
-
- if (unlink(objPath.c_str()) != 0) {
- ALOGE("Unable to remove the invalid cache file: %s. (reason: %s)\n",
- objPath.c_str(), strerror(errno));
- }
-
- infoFile.truncate();
- infoFile.close();
-
- if (unlink(infoPath.c_str()) != 0) {
- ALOGE("Unable to remove the invalid cache file: %s. (reason: %s)\n",
- infoPath.c_str(), strerror(errno));
- }
+ if (::unlink(infoPath.c_str()) != 0) {
+ ALOGE("Unable to remove the invalid cache file: %s. (reason: %s)\n",
+ infoPath.c_str(), ::strerror(errno));
}
}
- }
+ } // isCacheable()
return 0;
}
-
char const *RSScript::getCompilerErrorMessage() {
if (mStatus != ScriptStatus::Compiled) {
mErrorCode = BCC_INVALID_OPERATION;