Skip to content

Commit

Permalink
Added saftey for ensuring callback is called
Browse files Browse the repository at this point in the history
Summary:
Adds a return value to `MessageQueueThread#runOnQueue`, it's implementors and one caller.

It is currently possible for a callback to not be called (because the HandlerThread has been shutdown). If the callback is mission-critical (frees resources, releases a lock, etc) the callee has no indication that it needs to do something.

This surfaces that information to the callee.

Changelog:
[Android][Changed] - Made `MessageQueueThread#runOnQueue` return a boolean. Made `MessageQueueThreadImpl#runOnQueue` return false when the runnable is not submitted.

Reviewed By: RSNara

Differential Revision: D33075516

fbshipit-source-id: bd214a39ae066c0e7d413f2eaaaa05bd072ead2a
  • Loading branch information
MrMannWood authored and facebook-github-bot committed Dec 15, 2021
1 parent 19174a5 commit 89faf0c
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ public boolean isOnJSQueueThread() {
return Assertions.assertNotNull(mJSMessageQueueThread).isOnThread();
}

public void runOnJSQueueThread(Runnable runnable) {
Assertions.assertNotNull(mJSMessageQueueThread).runOnQueue(runnable);
public boolean runOnJSQueueThread(Runnable runnable) {
return Assertions.assertNotNull(mJSMessageQueueThread).runOnQueue(runnable);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public interface MessageQueueThread {
* if it is being submitted from the same queue Thread.
*/
@DoNotStrip
void runOnQueue(Runnable runnable);
boolean runOnQueue(Runnable runnable);

/**
* Runs the given Callable on this Thread. It will be submitted to the end of the event queue even
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@ private MessageQueueThreadImpl(
*/
@DoNotStrip
@Override
public void runOnQueue(Runnable runnable) {
public boolean runOnQueue(Runnable runnable) {
if (mIsFinished) {
FLog.w(
ReactConstants.TAG,
"Tried to enqueue runnable on already finished thread: '"
+ getName()
+ "... dropping Runnable.");
return false;
}
mHandler.post(runnable);
return true;
}

@DoNotStrip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void JMessageQueueThread::runOnQueue(std::function<void()> &&runnable) {
jni::ThreadScope guard;
static auto method =
JavaMessageQueueThread::javaClassStatic()
->getMethod<void(Runnable::javaobject)>("runOnQueue");
->getMethod<jboolean(Runnable::javaobject)>("runOnQueue");
method(
m_jobj,
JNativeRunnable::newObjectCxxArgs(wrapRunnable(std::move(runnable)))
Expand Down

0 comments on commit 89faf0c

Please sign in to comment.