Protect against crashes when over-releasing a TouchEvent

Summary:
It seems that, possibly due to optimizations and refactoring elsewhere in the event system, some TouchEvents are being over-disposed.

This doesn't really pose a problem besides performance; and could even indicate that an Event was in a pool but never properly initialized. In these
cases it seems perfectly reasonable to silently continue, and to log a soft exception. This WILL still crash in debug mode, so we can gather more information
if we find a good repro; otherwise we will continue to get production data from this soft exception if it's an issue and we can investigate further.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D30061178

fbshipit-source-id: 05d1f60afc382ce0a202ac8f3de34770cf9a760d
This commit is contained in:
Joshua Gross
2021-08-02 20:46:24 -07:00
committed by Facebook GitHub Bot
parent d9a9ae38d1
commit 402eec8de7
@@ -131,9 +131,25 @@ public class TouchEvent extends Event<TouchEvent> {
@Override
public void onDispose() {
Assertions.assertNotNull(mMotionEvent).recycle();
MotionEvent motionEvent = mMotionEvent;
mMotionEvent = null;
EVENTS_POOL.release(this);
if (motionEvent != null) {
motionEvent.recycle();
}
// Either `this` is in the event pool, or motionEvent
// is null. It is in theory not possible for a TouchEvent to
// be in the EVENTS_POOL but for motionEvent to be null. However,
// out of an abundance of caution and to avoid memory leaks or
// other crashes at all costs, we attempt to release here and log
// a soft exception here if release throws an IllegalStateException
// due to `this` being over-released. This may indicate that there is
// a logic error in our events system or pooling mechanism.
try {
EVENTS_POOL.release(this);
} catch (IllegalStateException e) {
ReactSoftException.logSoftException(TAG, e);
}
}
@Override