Schedule IWebSocketDelegate event calls on inspector thread (#45849)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45849

This fixes a bug where we were calling `delegate.didReceiveMessage` (and other handlers) from multiple threads on Android. In particular, with the addition of `Network.loadNetworkResource` in D54496969, we observed memory access issues in the implementation for `IO.read` in `NetworkIOAgent` after multiple successive requests are received.

This approach updates the Android-specific implementation of `IWebSocketDelegate` to schedule delegate handler and `close` calls on the inspector thread.

Changelog: [Internal]

Differential Revision: D60520747

fbshipit-source-id: 459b44b424157793faaf5967435e1303a0061292
This commit is contained in:
Alex Hunt
2024-08-07 09:08:28 -07:00
committed by Facebook GitHub Bot
parent 7cd1787e02
commit db3069b7ca
3 changed files with 39 additions and 15 deletions
@@ -101,24 +101,43 @@ import okhttp3.WebSocketListener;
new WebSocketListener() {
@Override
public void onFailure(WebSocket _unused, Throwable t, @Nullable Response response) {
@Nullable String message = t.getMessage();
delegate.didFailWithError(null, message != null ? message : "<Unknown error>");
// "No further calls to this listener will be made." -OkHttp docs for
// WebSocketListener.onFailure
delegate.close();
scheduleCallback(
new Runnable() {
public void run() {
@Nullable String message = t.getMessage();
delegate.didFailWithError(
null, message != null ? message : "<Unknown error>");
// "No further calls to this listener will be made." -OkHttp docs for
// WebSocketListener.onFailure
delegate.close();
}
},
0);
}
@Override
public void onMessage(WebSocket _unused, String text) {
delegate.didReceiveMessage(text);
scheduleCallback(
new Runnable() {
public void run() {
delegate.didReceiveMessage(text);
}
},
0);
}
@Override
public void onClosed(WebSocket _unused, int code, String reason) {
delegate.didClose();
// "No further calls to this listener will be made." -OkHttp docs for
// WebSocketListener.onClosed
delegate.close();
scheduleCallback(
new Runnable() {
public void run() {
delegate.didClose();
// "No further calls to this listener will be made." -OkHttp docs for
// WebSocketListener.onClosed
delegate.close();
}
},
0);
}
});
return new IWebSocket() {
@@ -71,11 +71,10 @@ class InspectorPackagerConnectionDelegate {
/**
* Schedules a function to run after a delay. If the function is called
* asynchronously, the implementer of InspectorPackagerConnectionDelegate
* is responsible for thread safety (e.g. scheduling the callback on a thread
* that has unique access to the InspectorPackagerConnection instance, or
* otherwise ensuring synchronization). The callback MAY be dropped and never
* called if no further callbacks are being accepted, e.g. if the application
* is terminating.
* is responsible for thread safety and should schedule the callback on
* the inspector queue. The callback MAY be dropped and never called if no
* further callbacks are being accepted, e.g. if the application is
* terminating.
*/
virtual void scheduleCallback(
std::function<void(void)> callback,
@@ -39,6 +39,8 @@ class IWebSocketDelegate {
/**
* Called when the socket has encountered an error.
* This method must be called on the inspector queue, and the
* WebSocketDelegate may not be destroyed while it is executing.
* \param posixCode POSIX errno value if available, otherwise nullopt.
* \param error Error description.
*/
@@ -48,6 +50,8 @@ class IWebSocketDelegate {
/**
* Called when a message has been received from the socket.
* This method must be called on the inspector queue, and the
* WebSocketDelegate may not be destroyed while it is executing.
* \param message Message received, in UTF-8 encoding.
*/
virtual void didReceiveMessage(std::string_view message) = 0;
@@ -55,6 +59,8 @@ class IWebSocketDelegate {
/**
* Called when the socket has been closed. The call is not required if
* didFailWithError was called instead.
* This method must be called on the inspector queue, and the
* WebSocketDelegate may not be destroyed while it is executing.
*/
virtual void didClose() = 0;
};