Fix: when calling Throwable.printStackTrace, logs from other threads may be inserted.#6487
Fix: when calling Throwable.printStackTrace, logs from other threads may be inserted.#6487valord577 wants to merge 2 commits intoGenymobile:devfrom
Throwable.printStackTrace, logs from other threads may be inserted.#6487Conversation
…s may be inserted. Signed-off-by: valord577 <valord577@gmail.com>
|
@rom1v Hi, I am new here. Do you have time to review this PR? |
|
Thank you for the PR. 👍 Indeed, I have noticed that some bug reports contain interleaved error messages (but I could never reproduced). I am wondering, is it really necessary to use an intermediate I think this should also work: diff --git server/src/main/java/com/genymobile/scrcpy/util/Ln.java server/src/main/java/com/genymobile/scrcpy/util/Ln.java
index 46057b4d3..b9b548afc 100644
--- server/src/main/java/com/genymobile/scrcpy/util/Ln.java
+++ server/src/main/java/com/genymobile/scrcpy/util/Ln.java
@@ -77,9 +77,7 @@ public final class Ln {
synchronized (CONSOLE_ERR) {
CONSOLE_ERR.print(PREFIX + "ERROR: " + message + '\n');
if (throwable != null) {
- StringWriter sw = new StringWriter();
- throwable.printStackTrace(new PrintWriter(sw));
- CONSOLE_ERR.print(sw.toString());
+ throwable.printStackTrace(CONSOLE_ERR);
}
}
}
@@ -95,9 +93,7 @@ public final class Ln {
synchronized (CONSOLE_ERR) {
CONSOLE_ERR.print(PREFIX + "ERROR: " + message + '\n');
if (throwable != null) {
- StringWriter sw = new StringWriter();
- throwable.printStackTrace(new PrintWriter(sw));
- CONSOLE_ERR.print(sw.toString());
+ throwable.printStackTrace(CONSOLE_ERR);
}
}
} |
In fact, this should be a different problem, since private void printStackTrace(PrintStreamOrWriter s) {
// Guard against malicious overrides of Throwable.equals by
// using a Set with identity equality semantics.
Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<>());
dejaVu.add(this);
synchronized (s.lock()) {
// Print our stack trace
s.println(this);
StackTraceElement[] trace = getOurStackTrace();
for (StackTraceElement traceElement : trace)
s.println("\tat " + traceElement);
// Print suppressed exceptions, if any
for (Throwable se : getSuppressed())
se.printEnclosedStackTrace(s, trace, SUPPRESSED_CAPTION, "\t", dejaVu);
// Print cause, if any
Throwable ourCause = getCause();
if (ourCause != null)
ourCause.printEnclosedStackTrace(s, trace, CAUSE_CAPTION, "", dejaVu);
}
}Adding synchronization would only avoid another message inserted between the |
|
Synchronization locks only occur when errors are reported. Such situations should be avoided whenever possible. Therefore, the impact on performance should be minimal. |
It is necessary to use a string concatenation class to collect stack trace information. Otherwise, it will be printed directly to stderr/stdout. private void printStackTrace(PrintStreamOrWriter s) {
Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<>());
dejaVu.add(this);
synchronized (s.lock()) {
s.println(this);
StackTraceElement[] trace = getOurStackTrace();
for (StackTraceElement traceElement : trace)
s.println("\tat " + traceElement);
// ^^^ Print message directly to stderr/stdout
for (Throwable se : getSuppressed())
se.printEnclosedStackTrace(s, trace, SUPPRESSED_CAPTION, "\t", dejaVu);
// ^^^ Print message directly to stderr/stdout
Throwable ourCause = getCause();
if (ourCause != null)
ourCause.printEnclosedStackTrace(s, trace, CAUSE_CAPTION, "", dejaVu);
// ^^^ Print message directly to stderr/stdout
}
} |
…s may be inserted. Signed-off-by: valord577 <valord577@gmail.com>
|
LGTM. I change the code! 👍 |
Between the calls to CONSOLE_ERR.print() and printStackTrace(CONSOLE_ERR), logs from other threads may be inserted. Synchronizing access to CONSOLE_ERR ensures that logs from different threads do not mix. PR #6487 <#6487> Signed-off-by: valord577 <valord577@gmail.com> Signed-off-by: Romain Vimont <rom@rom1v.com>
|
I squashed your two commits and updated the commit message: 7dd9bca Does it look ok to you? |
|
LGTM. No problem. |
|
Merged into |
scrcpy v3.3.4 Changes since v3.3.3: - Fix permission denial error after Android upgrade (Genymobile#6523) - Fix state restoration on certain devices (Genymobile#6405, Genymobile#6540) - Fix UHID_OUTPUT message parsing (Genymobile#6415) - Fix failure when the uniqueId field is missing on certain devices (Genymobile#6461) - Fix error log interleaving (Genymobile#6487) - Fix startup issue on certain Meizu devices (Genymobile#6480) - Fix handling of non-integer ANDROID_PLATFORM in build script (Genymobile#6408)

Fix: when calling
Throwable.printStackTrace, logs from other threads may be inserted.