Fix NPE in InPlaceEditView.reLayoutEdit #4235
Conversation
InPlaceEditView.reLayoutEdit accesses sInstance.impl within a Runnable posted to callSerially. If sInstance is cleared (e.g. by stopEdit) before this Runnable executes or inside it, sInstance.impl can throw a NPE or crash if sInstance is null. This change captures sInstance to a local variable and checks for nullity before accessing its members. Also added a reproduction test case in HelloCodenameOneInstrumentedTest.java.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
InPlaceEditView.reLayoutEdit accesses sInstance.impl within a Runnable posted to callSerially. If sInstance is cleared (e.g. by stopEdit) before this Runnable executes or inside it, sInstance.impl can throw a NPE. This change captures sInstance to a local variable and checks for nullity before accessing its members. Also added a reproduction test case in scripts/hellocodenameone using a NativeInterface to reproduce the race condition on Android. Deleted unused dead code HelloCodenameOneInstrumentedTest.java.
InPlaceEditView.reLayoutEdit accesses sInstance.impl within a Runnable posted to callSerially. If sInstance is cleared (e.g. by stopEdit) before this Runnable executes or inside it, sInstance.impl can throw a NPE. This change captures sInstance to a local variable and checks for nullity before accessing its members. Also added a reproduction test case in scripts/hellocodenameone using a NativeInterface to reproduce the race condition on Android. Deleted unused dead code HelloCodenameOneInstrumentedTest.java. Added iOS stub for the reproduction test. Updated test to avoid blocking EDT during wait.
Android screenshot updatesCompared 28 screenshots: 0 matched, 3 updated, 25 missing references.
Native Android coverage
|
|
I have addressed the timeout issue by moving the wait logic to a separate thread to avoid blocking the EDT. I also added the missing iOS native stub to ensure the test passes (skips) on iOS. |
InPlaceEditView.reLayoutEdit accesses sInstance.impl within a Runnable posted to callSerially. If sInstance is cleared (e.g. by stopEdit) before this Runnable executes or inside it, sInstance.impl can throw a NPE. This change captures sInstance to a local variable and checks for nullity before accessing its members. Also added a reproduction test case in scripts/hellocodenameone using a NativeInterface to reproduce the race condition on Android. Deleted unused dead code HelloCodenameOneInstrumentedTest.java. Added iOS stub (ObjC) for the reproduction test to avoid compilation errors. Updated test to avoid blocking EDT during wait.
Restored scripts/hellocodenameone/ios/src/main/java/com/codenameone/examples/hellocodenameone/InPlaceEditViewNativeImpl.java which is required for NativeLookup on iOS, complementing the Objective-C implementation. Confirmed Android implementation exists.
Removed the iOS Java stub and Objective-C implementation files. InPlaceEditViewNativeImpl should only exist in the Android subproject. On iOS, NativeLookup will simply not find the implementation, which is the desired behavior (skipping the test). This also avoids compilation issues related to missing native headers on iOS when the Java class is present.
Updated InPlaceEditViewNativeImpl (Android) to use direct imports (removing reflection) and a callback for result reporting. Added ReproductionCallback interface. Updated iOS stub and ObjC files to match the new signature. Adapted BaseTest and Cn1ssDeviceRunner to support explicit test failure reporting and screenshot status logging. InPlaceEditViewTest now waits for the callback and fails if an error occurs during reproduction.
iOS screenshot updatesCompared 28 screenshots: 0 matched, 3 updated, 25 missing references.
|
|
Acknowledged. The iOS implementation was removed to skip the test on iOS, so this should represent a clean state/skip. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Fixed a NullPointerException in
InPlaceEditView.javacaused by a race condition wheresInstancecould become null whilereLayoutEditwas executing asynchronously. Added a regression testtestInPlaceEditViewNPEinHelloCodenameOneInstrumentedTest.java.PR created automatically by Jules for task 3531270923084096351 started by @shai-almog