diff options
-rw-r--r-- | cmds/dumpstate/dumpstate.cpp | 104 | ||||
-rw-r--r-- | cmds/dumpstate/dumpstate.h | 8 | ||||
-rw-r--r-- | cmds/dumpstate/utils.cpp | 20 | ||||
-rw-r--r-- | cmds/installd/commands.cpp | 4 | ||||
-rw-r--r-- | cmds/installd/installd.cpp | 2 | ||||
-rw-r--r-- | cmds/installd/utils.cpp | 4 | ||||
-rw-r--r-- | data/etc/android.hardware.sensor.ambient_temperature.xml | 20 | ||||
-rw-r--r-- | data/etc/android.hardware.sensor.relative_humidity.xml | 20 | ||||
-rw-r--r-- | include/binder/Status.h | 41 | ||||
-rw-r--r-- | libs/binder/Status.cpp | 71 | ||||
-rw-r--r-- | services/surfaceflinger/DispSync.cpp | 11 | ||||
-rw-r--r-- | services/surfaceflinger/DispSync.h | 2 | ||||
-rw-r--r-- | services/surfaceflinger/Layer.cpp | 8 |
13 files changed, 219 insertions, 96 deletions
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 8b04b01e32..812d6fa3b3 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -17,8 +17,10 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> +#include <libgen.h> #include <limits.h> #include <memory> +#include <regex> #include <stdbool.h> #include <stdio.h> #include <stdlib.h> @@ -747,46 +749,57 @@ int main(int argc, char *argv[]) { redirect_to_socket(stdout, "dumpstate"); } - /* redirect output if needed */ - std::string text_path, zip_path, tmp_path, entry_name; + /* full path of the directory where the bug report files will be written */ + std::string bugreport_dir; + + /* full path of the temporary file containing the bug report */ + std::string tmp_path; + + /* base name (without suffix or extensions) of the bug report files */ + std::string base_name; + + /* suffix of the bug report files - it's typically the date (when invoked with -d), + * although it could be changed by the user using a system property */ + std::string suffix; /* pointer to the actual path, be it zip or text */ std::string path; time_t now = time(NULL); + /* redirect output if needed */ bool is_redirecting = !use_socket && use_outfile; if (is_redirecting) { - text_path = use_outfile; + bugreport_dir = dirname(use_outfile); + base_name = basename(use_outfile); if (do_add_date) { char date[80]; - strftime(date, sizeof(date), "-%Y-%m-%d-%H-%M-%S", localtime(&now)); - text_path += date; + strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&now)); + suffix = date; + } else { + suffix = "undated"; } if (do_fb) { - screenshot_path = text_path + ".png"; + // TODO: if dumpstate was an object, the paths could be internal variables and then + // we could have a function to calculate the derived values, such as: + // screenshot_path = GetPath(".png"); + screenshot_path = bugreport_dir + "/" + base_name + "-" + suffix + ".png"; } - zip_path = text_path + ".zip"; - text_path += ".txt"; - tmp_path = text_path + ".tmp"; - entry_name = basename(text_path.c_str()); + tmp_path = bugreport_dir + "/" + base_name + "-" + suffix + ".tmp"; - ALOGD("Temporary path: %s\ntext path: %s\nzip path: %s\nzip entry: %s", - tmp_path.c_str(), text_path.c_str(), zip_path.c_str(), entry_name.c_str()); + ALOGD("Bugreport dir: %s\nBase name: %s\nSuffix: %s\nTemporary path: %s\n" + "Screenshot path: %s\n", bugreport_dir.c_str(), base_name.c_str(), suffix.c_str(), + tmp_path.c_str(), screenshot_path.c_str()); if (do_update_progress) { - if (!entry_name.empty()) { - std::vector<std::string> am_args = { - "--receiver-permission", "android.permission.DUMP", - "--es", "android.intent.extra.NAME", entry_name, - "--ei", "android.intent.extra.PID", std::to_string(getpid()), - "--ei", "android.intent.extra.MAX", std::to_string(WEIGHT_TOTAL), - }; - send_broadcast("android.intent.action.BUGREPORT_STARTED", am_args); - } else { - ALOGE("Skipping started broadcast because entry name could not be inferred\n"); - } + std::vector<std::string> am_args = { + "--receiver-permission", "android.permission.DUMP", + "--es", "android.intent.extra.NAME", suffix, + "--ei", "android.intent.extra.PID", std::to_string(getpid()), + "--ei", "android.intent.extra.MAX", std::to_string(WEIGHT_TOTAL), + }; + send_broadcast("android.intent.action.BUGREPORT_STARTED", am_args); } } @@ -852,8 +865,6 @@ int main(int argc, char *argv[]) { } if (is_redirecting) { - ALOGD("Temporary path: %s\ntext path: %s\nzip path: %s\nzip entry: %s", - tmp_path.c_str(), text_path.c_str(), zip_path.c_str(), entry_name.c_str()); /* TODO: rather than generating a text file now and zipping it later, it would be more efficient to redirect stdout to the zip entry directly, but the libziparchive doesn't support that option yet. */ @@ -877,10 +888,42 @@ int main(int argc, char *argv[]) { /* rename or zip the (now complete) .tmp file to its final location */ if (use_outfile) { + + /* check if user changed the suffix using system properties */ + char key[PROPERTY_KEY_MAX]; + char value[PROPERTY_VALUE_MAX]; + sprintf(key, "dumpstate.%d.name", getpid()); + property_get(key, value, ""); + bool change_suffix= false; + if (value[0]) { + /* must whitelist which characters are allowed, otherwise it could cross directories */ + std::regex valid_regex("^[-_a-zA-Z0-9]+$"); + if (std::regex_match(value, valid_regex)) { + change_suffix = true; + } else { + ALOGE("invalid suffix provided by user: %s", value); + } + } + if (change_suffix) { + ALOGI("changing suffix from %s to %s", suffix.c_str(), value); + suffix = value; + if (!screenshot_path.empty()) { + std::string new_screenshot_path = + bugreport_dir + "/" + base_name + "-" + suffix + ".png"; + if (rename(screenshot_path.c_str(), new_screenshot_path.c_str())) { + ALOGE("rename(%s, %s): %s\n", screenshot_path.c_str(), + new_screenshot_path.c_str(), strerror(errno)); + } else { + screenshot_path = new_screenshot_path; + } + } + } + bool do_text_file = true; if (do_zip_file) { - path = zip_path; - if (!generate_zip_file(tmp_path, zip_path, entry_name, now)) { + ALOGD("Generating .zip bugreport"); + path = bugreport_dir + "/" + base_name + "-" + suffix + ".zip"; + if (!generate_zip_file(tmp_path, path, base_name + "-" + suffix + ".txt", now)) { ALOGE("Failed to generate zip file; sending text bugreport instead\n"); do_text_file = true; } else { @@ -888,9 +931,10 @@ int main(int argc, char *argv[]) { } } if (do_text_file) { - path = text_path; - if (rename(tmp_path.c_str(), text_path.c_str())) { - ALOGE("rename(%s, %s): %s\n", tmp_path.c_str(), text_path.c_str(), strerror(errno)); + ALOGD("Generating .txt bugreport"); + path = bugreport_dir + "/" + base_name + "-" + suffix + ".txt"; + if (rename(tmp_path.c_str(), path.c_str())) { + ALOGE("rename(%s, %s): %s\n", tmp_path.c_str(), path.c_str(), strerror(errno)); path.clear(); } } diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 2ba5ccb116..c7e7f52464 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -51,8 +51,12 @@ typedef void (for_each_tid_func)(int, int, const char *); * can be calculated by dividing the all completed weight by the total weight. * * This value is defined empirically and it need to be adjusted as more sections are added. - * It does not need to match the exact sum of all sections, but it should to be more than it, - * otherwise the calculated progress would be more than 100%. + * + * It does not need to match the exact sum of all sections, but ideally it should to be slight more + * than such sum: a value too high will cause the bugreport to finish before the user expected (for + * example, jumping from 70% to 100%), while a value too low will cause the progress to fluctuate + * down (for example, from 70% to 50%, since the actual max will be automatically increased every + * time it is reached). */ static const int WEIGHT_TOTAL = 4000; diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index 8683155199..a7637d83e7 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -553,7 +553,7 @@ int run_command_always(const char *title, int timeout_seconds, const char *args[ void send_broadcast(const std::string& action, const std::vector<std::string>& args) { if (args.size() > 1000) { - fprintf(stderr, "send_broadcast: too many arguments (%d)\n", args.size()); + fprintf(stderr, "send_broadcast: too many arguments (%d)\n", (int) args.size()); return; } const char *am_args[1024] = { "/system/bin/am", "broadcast", "--user", "0", @@ -841,6 +841,7 @@ void dump_route_tables() { /* overall progress */ int progress = 0; int do_update_progress = 0; // Set by dumpstate.cpp +int weight_total = WEIGHT_TOTAL; // TODO: make this function thread safe if sections are generated in parallel. void update_progress(int delta) { @@ -850,12 +851,27 @@ void update_progress(int delta) { char key[PROPERTY_KEY_MAX]; char value[PROPERTY_VALUE_MAX]; + + // adjusts max on the fly + if (progress > weight_total) { + int new_total = weight_total * 1.2; + fprintf(stderr, "Adjusting total weight from %d to %d\n", weight_total, new_total); + weight_total = new_total; + sprintf(key, "dumpstate.%d.max", getpid()); + sprintf(value, "%d", weight_total); + int status = property_set(key, value); + if (status) { + ALOGW("Could not update max weight by setting system property %s to %s: %d\n", + key, value, status); + } + } + sprintf(key, "dumpstate.%d.progress", getpid()); sprintf(value, "%d", progress); // stderr is ignored on normal invocations, but useful when calling /system/bin/dumpstate // directly for debuggging. - fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, WEIGHT_TOTAL); + fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, weight_total); int status = property_set(key, value); if (status) { diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp index 17201d5563..3b2a086487 100644 --- a/cmds/installd/commands.cpp +++ b/cmds/installd/commands.cpp @@ -16,8 +16,8 @@ #include "installd.h" -#include <base/stringprintf.h> -#include <base/logging.h> +#include <android-base/stringprintf.h> +#include <android-base/logging.h> #include <cutils/sched_policy.h> #include <diskusage/dirsize.h> #include <logwrap/logwrap.h> diff --git a/cmds/installd/installd.cpp b/cmds/installd/installd.cpp index cb76f33fd8..a7202b63db 100644 --- a/cmds/installd/installd.cpp +++ b/cmds/installd/installd.cpp @@ -16,7 +16,7 @@ #include "installd.h" -#include <base/logging.h> +#include <android-base/logging.h> #include <sys/capability.h> #include <sys/prctl.h> diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp index e51e731e15..750a136f60 100644 --- a/cmds/installd/utils.cpp +++ b/cmds/installd/utils.cpp @@ -16,8 +16,8 @@ #include "installd.h" -#include <base/stringprintf.h> -#include <base/logging.h> +#include <android-base/stringprintf.h> +#include <android-base/logging.h> #define CACHE_NOISY(x) //x diff --git a/data/etc/android.hardware.sensor.ambient_temperature.xml b/data/etc/android.hardware.sensor.ambient_temperature.xml new file mode 100644 index 0000000000..8ad140ea73 --- /dev/null +++ b/data/etc/android.hardware.sensor.ambient_temperature.xml @@ -0,0 +1,20 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2009 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<!-- Feature for devices with an ambient temperature sensor. --> +<permissions> + <feature name="android.hardware.sensor.ambient_temperature" /> +</permissions> diff --git a/data/etc/android.hardware.sensor.relative_humidity.xml b/data/etc/android.hardware.sensor.relative_humidity.xml new file mode 100644 index 0000000000..b9432d34a2 --- /dev/null +++ b/data/etc/android.hardware.sensor.relative_humidity.xml @@ -0,0 +1,20 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2009 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<!-- Feature for devices with a relative humidity sensor. --> +<permissions> + <feature name="android.hardware.sensor.relative_humidity" /> +</permissions> diff --git a/include/binder/Status.h b/include/binder/Status.h index 04738f8dd1..d19824d894 100644 --- a/include/binder/Status.h +++ b/include/binder/Status.h @@ -60,31 +60,31 @@ public: EX_ILLEGAL_STATE = -5, EX_NETWORK_MAIN_THREAD = -6, EX_UNSUPPORTED_OPERATION = -7, - EX_TRANSACTION_FAILED = -8, // This is special and Java specific; see Parcel.java. EX_HAS_REPLY_HEADER = -128, + // This is special, and indicates to C++ binder proxies that the + // transaction has failed at a low level. + EX_TRANSACTION_FAILED = -129, }; + // A more readable alias for the default constructor. + static Status ok(); // Allow authors to explicitly pick whether their integer is a status_t or // exception code. - static Status fromExceptionCode(int32_t exception_code); + static Status fromExceptionCode(int32_t exceptionCode); + static Status fromExceptionCode(int32_t exceptionCode, + const String8& message); static Status fromStatusT(status_t status); - // A more readable alias for the default constructor. - static Status ok(); Status() = default; - Status(int32_t exception_code, const String8& message); - Status(int32_t exception_code, const char* message); - + ~Status() = default; // Status objects are copyable and contain just simple data. Status(const Status& status) = default; Status(Status&& status) = default; Status& operator=(const Status& status) = default; - ~Status() = default; - // Bear in mind that if the client or service is a Java endpoint, this // is not the logic which will provide/interpret the data here. status_t readFromParcel(const Parcel& parcel); @@ -92,16 +92,17 @@ public: // Set one of the pre-defined exception types defined above. void setException(int32_t ex, const String8& message); - // A few of the status_t values map to exception codes, but most of them - // simply map to "transaction failed." + // Setting a |status| != OK causes generated code to return |status| + // from Binder transactions, rather than writing an exception into the + // reply Parcel. void setFromStatusT(status_t status); // Get information about an exception. - // Any argument may be given as nullptr. - void getException(int32_t* returned_exception, - String8* returned_message) const; int32_t exceptionCode() const { return mException; } const String8& exceptionMessage() const { return mMessage; } + status_t transactionError() const { + return mException == EX_TRANSACTION_FAILED ? mErrorCode : OK; + } bool isOk() const { return mException == EX_NONE; } @@ -109,9 +110,17 @@ public: String8 toString8() const; private: - // We always write |mException| to the parcel. - // If |mException| != EX_NONE, we write message as well. + Status(int32_t exception_code); + Status(int32_t exception_code, const String8& message); + + // If |mException| == EX_TRANSACTION_FAILED, generated code will return + // |mErrorCode| as the result of the transaction rather than write an + // exception to the reply parcel. + // + // Otherwise, we always write |mException| to the parcel. + // If |mException| != EX_NONE, we write |mMessage| as well. int32_t mException = EX_NONE; + int32_t mErrorCode = 0; String8 mMessage; }; // class Status diff --git a/libs/binder/Status.cpp b/libs/binder/Status.cpp index 41fff3d4cd..67f0d59669 100644 --- a/libs/binder/Status.cpp +++ b/libs/binder/Status.cpp @@ -19,8 +19,17 @@ namespace android { namespace binder { -Status Status::fromExceptionCode(int32_t exception_code) { - return Status(exception_code, ""); +Status Status::ok() { + return Status(); +} + +Status Status::fromExceptionCode(int32_t exceptionCode) { + return Status(exceptionCode); +} + +Status Status::fromExceptionCode(int32_t exceptionCode, + const String8& message) { + return Status(exceptionCode, message); } Status Status::fromStatusT(status_t status) { @@ -29,16 +38,9 @@ Status Status::fromStatusT(status_t status) { return ret; } -Status Status::ok() { - return Status(); -} - -Status::Status(int32_t exception_code, const String8& message) - : mException(exception_code), - mMessage(message) {} - -Status::Status(int32_t exception_code, const char* message) - : mException(exception_code), +Status::Status(int32_t exceptionCode) : mException(exceptionCode) {} +Status::Status(int32_t exceptionCode, const String8& message) + : mException(exceptionCode), mMessage(message) {} status_t Status::readFromParcel(const Parcel& parcel) { @@ -69,12 +71,24 @@ status_t Status::readFromParcel(const Parcel& parcel) { } // The remote threw an exception. Get the message back. - mMessage = String8(parcel.readString16()); + String16 message; + status = parcel.readString16(&message); + if (status != OK) { + setFromStatusT(status); + return status; + } + mMessage = String8(message); return status; } status_t Status::writeToParcel(Parcel* parcel) const { + // Something really bad has happened, and we're not going to even + // try returning rich error data. + if (mException == EX_TRANSACTION_FAILED) { + return mErrorCode; + } + status_t status = parcel->writeInt32(mException); if (status != OK) { return status; } if (mException == EX_NONE) { @@ -86,43 +100,26 @@ status_t Status::writeToParcel(Parcel* parcel) const { } void Status::setFromStatusT(status_t status) { - switch (status) { - case NO_ERROR: - mException = EX_NONE; - mMessage.clear(); - break; - case UNEXPECTED_NULL: - mException = EX_NULL_POINTER; - mMessage.setTo("Unexpected null reference in Parcel"); - break; - default: - mException = EX_TRANSACTION_FAILED; - mMessage.setTo("Transaction failed"); - break; - } + mException = (status == NO_ERROR) ? EX_NONE : EX_TRANSACTION_FAILED; + mErrorCode = status; + mMessage.clear(); } void Status::setException(int32_t ex, const String8& message) { mException = ex; + mErrorCode = NO_ERROR; // an exception, not a transaction failure. mMessage.setTo(message); } -void Status::getException(int32_t* returned_exception, - String8* returned_message) const { - if (returned_exception) { - *returned_exception = mException; - } - if (returned_message) { - returned_message->setTo(mMessage); - } -} - String8 Status::toString8() const { String8 ret; if (mException == EX_NONE) { ret.append("No error"); } else { ret.appendFormat("Status(%d): '", mException); + if (mException == EX_TRANSACTION_FAILED) { + ret.appendFormat("%d: ", mErrorCode); + } ret.append(String8(mMessage)); ret.append("'"); } diff --git a/services/surfaceflinger/DispSync.cpp b/services/surfaceflinger/DispSync.cpp index 5ba387d020..e0d7339df0 100644 --- a/services/surfaceflinger/DispSync.cpp +++ b/services/surfaceflinger/DispSync.cpp @@ -322,6 +322,7 @@ void DispSync::reset() { mNumResyncSamples = 0; mFirstResyncSample = 0; mNumResyncSamplesSincePresent = 0; + mNumPresentWithoutResyncSamples = 0; resetErrorLocked(); } @@ -346,6 +347,15 @@ bool DispSync::addPresentFence(const sp<Fence>& fence) { updateErrorLocked(); + // This is a workaround for b/25845510. + // If we have no resync samples after many presents, something is wrong with + // HW vsync. Tell SF to disable HW vsync now and re-enable it next time. + if (mNumResyncSamples == 0 && + mNumPresentWithoutResyncSamples++ > MAX_PRESENT_WITHOUT_RESYNC_SAMPLES) { + mNumPresentWithoutResyncSamples = 0; + return false; + } + return !mModelUpdated || mError > kErrorThreshold; } @@ -354,6 +364,7 @@ void DispSync::beginResync() { mModelUpdated = false; mNumResyncSamples = 0; + mNumPresentWithoutResyncSamples = 0; } bool DispSync::addResyncSample(nsecs_t timestamp) { diff --git a/services/surfaceflinger/DispSync.h b/services/surfaceflinger/DispSync.h index a8524b943c..1ee0865cee 100644 --- a/services/surfaceflinger/DispSync.h +++ b/services/surfaceflinger/DispSync.h @@ -140,6 +140,7 @@ private: enum { MIN_RESYNC_SAMPLES_FOR_UPDATE = 3 }; enum { NUM_PRESENT_SAMPLES = 8 }; enum { MAX_RESYNC_SAMPLES_WITHOUT_PRESENT = 4 }; + enum { MAX_PRESENT_WITHOUT_RESYNC_SAMPLES = 8 }; // mPeriod is the computed period of the modeled vsync events in // nanoseconds. @@ -168,6 +169,7 @@ private: size_t mFirstResyncSample; size_t mNumResyncSamples; int mNumResyncSamplesSincePresent; + int mNumPresentWithoutResyncSamples; // These member variables store information about the present fences used // to validate the currently computed model. diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 2b6e0ec740..d4847080c6 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -147,10 +147,6 @@ void Layer::onFirstRef() { } Layer::~Layer() { - sp<Client> c(mClientRef.promote()); - if (c != 0) { - c->detachLayer(this); - } mFlinger->deleteTextureAsync(mTextureName); mFrameTracker.logAndResetStats(mName); } @@ -252,6 +248,10 @@ void Layer::onSidebandStreamChanged() { // the layer has been remove from the current state list (and just before // it's removed from the drawing state list) void Layer::onRemoved() { + sp<Client> c(mClientRef.promote()); + if (c != 0) { + c->detachLayer(this); + } mSurfaceFlingerConsumer->abandon(); } |