Permalink
Browse files

Fail-Fast on Redundant Java Callback Invocations

Reviewed By: dcaspi

Differential Revision: D4354731

fbshipit-source-id: d23351f23c8c5962da60af77340166fcb1314d78
  • Loading branch information...
theoy authored and facebook-github-bot committed Dec 21, 2016
1 parent 8305743 commit e64618374b2e23ddd0ec91b777c2677c19ac5de3
Showing with 8 additions and 0 deletions.
  1. +8 −0 ReactAndroid/src/main/java/com/facebook/react/bridge/CallbackImpl.java
@@ -17,15 +17,23 @@
private final CatalystInstance mCatalystInstance;
private final ExecutorToken mExecutorToken;
private final int mCallbackId;
private boolean mInvoked;
public CallbackImpl(CatalystInstance bridge, ExecutorToken executorToken, int callbackId) {
mCatalystInstance = bridge;
mExecutorToken = executorToken;
mCallbackId = callbackId;
mInvoked = false;
}
@Override
public void invoke(Object... args) {
if (mInvoked) {
throw new RuntimeException("Illegal callback invocation from native "+
"module. This callback type only permits a single invocation from "+
"native code.");
}
mCatalystInstance.invokeCallback(mExecutorToken, mCallbackId, Arguments.fromJavaArgs(args));
mInvoked = true;
}
}

3 comments on commit e646183

@gavinyeah

This comment has been minimized.

Show comment
Hide comment
@gavinyeah

gavinyeah Apr 20, 2017

This change breaks some modules that invoke the callback multiple times. E.g. react-native-dialogs, on a Android dialog, we might invoke the callback multiple times to react to user actions like check the radio button (onSelection) and click ok (onPositive).

gavinyeah replied Apr 20, 2017

This change breaks some modules that invoke the callback multiple times. E.g. react-native-dialogs, on a Android dialog, we might invoke the callback multiple times to react to user actions like check the radio button (onSelection) and click ok (onPositive).

@ide

This comment has been minimized.

Show comment
Hide comment
@ide

ide Apr 21, 2017

Collaborator

@gavinyeah Does that even work? AFAIK the JS side frees a callback once it's been called. If you need to send back multiple messages from native to JS, use events instead of callbacks as your message-passing mechanism.

Collaborator

ide replied Apr 21, 2017

@gavinyeah Does that even work? AFAIK the JS side frees a callback once it's been called. If you need to send back multiple messages from native to JS, use events instead of callbacks as your message-passing mechanism.

@cbrevik

This comment has been minimized.

Show comment
Hide comment
@cbrevik

cbrevik May 19, 2017

Contributor

I'm seeing this intermittently with a third-party SDK I've wrapped that sometimes does a second callback.

Would at least make life a bit easier if Callback exposed Invoked, and that would in turn be checkable in Promise (which could expose its own value to signify if it has been "resolved" or "rejected" by checking the two underlying callbacks).

Contributor

cbrevik replied May 19, 2017

I'm seeing this intermittently with a third-party SDK I've wrapped that sometimes does a second callback.

Would at least make life a bit easier if Callback exposed Invoked, and that would in turn be checkable in Promise (which could expose its own value to signify if it has been "resolved" or "rejected" by checking the two underlying callbacks).

Please sign in to comment.