Handle export ended after internal thread is quit

Export can be ended twice if an exception is thrown before Transformer
was fully released.

Also add a lock to make sure the released variable doesn't become true
right after its value is checked.

PiperOrigin-RevId: 756196139
This commit is contained in:
kimvde 2025-05-08 01:05:29 -07:00 committed by Copybara-Service
parent db1c478dad
commit f5ff9672e2

View File

@ -148,6 +148,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
private final Object setMaxSequenceDurationUsLock; private final Object setMaxSequenceDurationUsLock;
private final Object progressLock; private final Object progressLock;
private final ProgressHolder internalProgressHolder; private final ProgressHolder internalProgressHolder;
private final Object releaseLock;
private final ImmutableList<Integer> allowedEncodingRotationDegrees; private final ImmutableList<Integer> allowedEncodingRotationDegrees;
private final int maxFramesInEncoder; private final int maxFramesInEncoder;
@ -181,10 +182,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
/** /**
* The boolean tracking if this component has been released. * The boolean tracking if this component has been released.
* *
* <p>Modified on the internal thread. Accessed on the application thread (in {@link #getProgress} * <p>Modified on the internal thread. Accessed on multiple threads. Writes on the internal thread
* and {@link #cancel()}). * and reads on other threads are guarded by releaseLock.
*/ */
private volatile boolean released; private boolean released;
public TransformerInternal( public TransformerInternal(
Context context, Context context,
@ -266,6 +267,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
canceledConditionVariable = new ConditionVariable(); canceledConditionVariable = new ConditionVariable();
progressLock = new Object(); progressLock = new Object();
internalProgressHolder = new ProgressHolder(); internalProgressHolder = new ProgressHolder();
releaseLock = new Object();
sampleExporters = new ArrayList<>(); sampleExporters = new ArrayList<>();
// It's safe to use "this" because we don't send a message before exiting the constructor. // It's safe to use "this" because we don't send a message before exiting the constructor.
@ -300,13 +302,15 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
} }
public void cancel() { public void cancel() {
if (released) { synchronized (releaseLock) {
return; if (released) {
return;
}
verifyInternalThreadAlive();
internalHandler
.obtainMessage(MSG_END, END_REASON_CANCELLED, /* unused */ 0, /* exportException */ null)
.sendToTarget();
} }
verifyInternalThreadAlive();
internalHandler
.obtainMessage(MSG_END, END_REASON_CANCELLED, /* unused */ 0, /* exportException */ null)
.sendToTarget();
clock.onThreadBlocked(); clock.onThreadBlocked();
canceledConditionVariable.blockUninterruptible(); canceledConditionVariable.blockUninterruptible();
canceledConditionVariable.close(); canceledConditionVariable.close();
@ -323,10 +327,16 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
} }
public void endWithException(ExportException exportException) { public void endWithException(ExportException exportException) {
verifyInternalThreadAlive(); synchronized (releaseLock) {
internalHandler if (released) {
.obtainMessage(MSG_END, END_REASON_ERROR, /* unused */ 0, exportException) Log.w(TAG, "Export error after export ended", exportException);
.sendToTarget(); return;
}
verifyInternalThreadAlive();
internalHandler
.obtainMessage(MSG_END, END_REASON_ERROR, /* unused */ 0, exportException)
.sendToTarget();
}
} }
// Private methods. // Private methods.
@ -405,7 +415,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
@Nullable ExportException releaseExportException = null; @Nullable ExportException releaseExportException = null;
boolean releasedPreviously = released; boolean releasedPreviously = released;
if (!released) { if (!released) {
released = true; synchronized (releaseLock) {
released = true;
}
Log.i( Log.i(
TAG, TAG,
@ -629,9 +641,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
@Nullable @Nullable
@Override @Override
public SampleConsumer onOutputFormat(Format assetLoaderOutputFormat) throws ExportException { public SampleConsumer onOutputFormat(Format assetLoaderOutputFormat) throws ExportException {
if (released) {
return null;
}
synchronized (assetLoaderLock) { synchronized (assetLoaderLock) {
if (!assetLoaderInputTracker.hasRegisteredAllTracks()) { if (!assetLoaderInputTracker.hasRegisteredAllTracks()) {
return null; return null;