Skip to content

arch/sim: Stop publishing stale X11 display during teardown.#18956

Merged
jerpelea merged 1 commit into
apache:masterfrom
Zepp-Hanzj:fix/sim-x11-teardown-display
May 27, 2026
Merged

arch/sim: Stop publishing stale X11 display during teardown.#18956
jerpelea merged 1 commit into
apache:masterfrom
Zepp-Hanzj:fix/sim-x11-teardown-display

Conversation

@Zepp-Hanzj
Copy link
Copy Markdown
Contributor

@Zepp-Hanzj Zepp-Hanzj commented May 25, 2026

Summary

  • Fixes a shutdown ordering issue in the sim X11 framebuffer teardown path.
  • sim_x11events() may continue polling g_display from the idle loop while the X11 connection is being torn down.
  • Save the current Display * in a local variable, clear the global g_display before teardown, and use the local pointer for the remaining X11 cleanup calls.
  • Related NuttX Issue: none.
  • Related NuttX Apps Issue/Pull Request: none.

Impact

  • Is new feature added? NO.
  • Is existing feature changed? YES: only the sim X11 framebuffer shutdown ordering is changed.
  • Impact on user? NO expected user-visible behavior change; the change avoids publishing a stale Display pointer during shutdown.
  • Impact on build? NO.
  • Impact on hardware? NO real hardware impact; this is limited to the simulator X11 framebuffer path.
  • Impact on documentation? NO.
  • Impact on security? NO.
  • Impact on compatibility? NO API, ABI, or configuration compatibility change.
  • Dependencies: none.

Testing

I confirm that changes are verified on local setup and work as intended:

  • Build Host(s): Linux DESKTOP-6B1686O 6.6.114.1-microsoft-standard-WSL2 x86_64, gcc (Ubuntu 10.5.0-1ubuntu1~20.04) 10.5.0.
  • Target(s): sim:nsh.
  • Static/style check: ./tools/checkpatch.sh -g HEAD~...HEAD.
  • Build test: ./tools/configure.sh sim:nsh && make -j4.
  • Runtime smoke test: booted ./nuttx, ran help, uname -a, and poweroff.

Testing logs before change:

Code inspection: before this change, sim_x11uninit() kept g_display published while
the teardown path called XShmDetach(), XUngrabButton(), and XCloseDisplay() through
g_display. sim_x11events() also polls g_display from the idle loop, so the event
path could observe a stale Display pointer during shutdown.

Testing logs after change:

$ ./tools/checkpatch.sh -g HEAD~...HEAD
✔️ All checks pass.

$ ./tools/configure.sh sim:nsh
# configuration written to .config

$ make -j4
LD:  nuttx
Pac SIM with dynamic libs..
SIM elf with dynamic libs archive in nuttx.tgz

$ printf 'help\nuname -a\npoweroff\n' | timeout 10s ./nuttx
NuttShell (NSH) NuttX-12.13.0
nsh> help
help usage:  help [-v] [<cmd>]
...
nsh> uname -a
NuttX 12.13.0 a1a048d9f6 May 25 2026 23:55:29 sim sim
nsh> poweroff

PR verification Self-Check

  • This PR introduces only one functional change.
  • I have updated all required description fields above.
  • My PR adheres to Contributing Guidelines and Documentation (git commit title and message, coding standard, etc).
  • My PR is still work in progress (not ready for review).
  • My PR is ready for review and can be safely merged into a codebase.

@github-actions github-actions Bot added Arch: simulator Issues related to the SIMulator Size: S The size of the change in this PR is small labels May 25, 2026
@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/sim-x11-teardown-display branch from b8372f4 to d714c02 Compare May 25, 2026 06:18
Copy link
Copy Markdown
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please follow the contribution guidelines and add a proper PR title, description and test cases

sim_x11events() polls g_display from the idle loop while the
framebuffer teardown path closes the X connection.  Clear the global
Display handle before teardown so the event path stops using it, but
keep the saved local Display pointer for XShmDetach(), XUngrabButton(),
and XCloseDisplay().

This only changes the sim X11 framebuffer shutdown ordering and does
not change user-visible APIs or build configuration.

Signed-off-by: hanzhijian <hanzhijian@zepp.com>
@Zepp-Hanzj Zepp-Hanzj changed the title arch/sim: stop publishing stale X11 display during teardown arch/sim: Stop publishing stale X11 display during teardown. May 25, 2026
@Zepp-Hanzj Zepp-Hanzj force-pushed the fix/sim-x11-teardown-display branch from d714c02 to a1a048d Compare May 25, 2026 15:56
@Zepp-Hanzj
Copy link
Copy Markdown
Contributor Author

Addressed the requested PR metadata/test details:

  • Amended the commit title/message to follow the contribution guideline format.
  • Updated the PR title and description with Summary, Impact, Testing, and Self-Check sections.
  • Added local validation details for ./tools/checkpatch.sh -g HEAD~...HEAD, ./tools/configure.sh sim:nsh && make -j4, and a ./nuttx runtime smoke test.

CI has been re-triggered on the updated commit.

@Zepp-Hanzj
Copy link
Copy Markdown
Contributor Author

@jerpelea The requested PR metadata/test details have been addressed in the current head commit:

  • PR title now follows the NuttX contribution guideline format.
  • PR description includes Summary, Impact, Testing, and Self-Check sections.
  • Testing details include checkpatch, sim:nsh build, and runtime smoke-test evidence.
  • Current CI is green.

Could you please re-review when convenient so the stale change-request state can be cleared if this now looks OK? Thanks.

@Zepp-Hanzj Zepp-Hanzj requested a review from jerpelea May 26, 2026 08:32
@jerpelea jerpelea merged commit 2969c88 into apache:master May 27, 2026
15 checks passed
@Zepp-Hanzj
Copy link
Copy Markdown
Contributor Author

🎉 Congratulations on the merge! Thank you for the contribution — glad to see this sim/X11 teardown fix landed successfully. Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: simulator Issues related to the SIMulator Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants