Skip to content

fix: harden screen selection in RvApplication against invalid indices (#1211)#1321

Open
pjurkas wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
pjurkas:fix-crash
Open

fix: harden screen selection in RvApplication against invalid indices (#1211)#1321
pjurkas wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
pjurkas:fix-crash

Conversation

@pjurkas

@pjurkas pjurkas commented Jun 30, 2026

Copy link
Copy Markdown

Linked issues

Fixes #1211

Summarize your change.

Harden screen selection in RvApplication against invalid QScreen indices and
null pointers so OpenRV no longer crashes (SIGSEGV in QScreen::geometry()) at
startup when multiple displays are connected.

Describe the reason for the change.

Issue #1211 reports a SIGSEGV in Rv::RvApplication::newSessionFromFiles
QScreen::geometry() on multi-monitor setups (RHEL 9.5). The crash happens
because the existing code dereferences QScreen* values that can be nullptr,
and indexes into QGuiApplication::screens() with values (from saved
preferences or -screen command-line options) that may be out of range or
stale. Under certain GNOME multi-display configurations the primary screen or
the requested screen index is not what the code assumes, and we crash.

Describe what you have tested and on which operating system.

Add a list of changes, and note any that might need special attention during the review.

src/lib/app/RvCommon/RvApplication.cpp:

  • Guard isVirtualDesktop against a null primary screen and null entries in
    QGuiApplication::screens().
  • Use QGuiApplication::screenAt() for more robust point-to-screen mapping,
    with a manual geometry fallback when it returns null.
  • Validate opts.screen against the current screen count before applying it,
    so stale preferences or out-of-bounds command-line values do not propagate.
  • Bounds-check screen indices and null-check QScreen* pointers before
    dereferencing geometry() in the relative-position adjustment path.

Reviewer focus: please double-check the fallback path when
QGuiApplication::screenAt() returns null and the chosen index when
opts.screen is invalid — those are the behavioral edges most likely to
affect existing multi-monitor workflows.

If possible, provide screenshots.

N/A — crash fix.

- Guard isVirtualDesktop against null primary screen and null screens
- Use QGuiApplication::screenAt() for more robust point-to-screen mapping,
  with a manual geometry fallback
- Validate opts.screen index before applying it, so stale preferences or
  out-of-bounds command-line values do not propagate
- Bounds-check screen indices and null-check QScreen pointers before
  dereferencing geometry() in the relative-position adjustment path
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 30, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: pjurkas / name: Pavel Jurkas (a7744cb)

QRect totalGeometry;
for (const auto& screen : screens)
{
if (screen == nullptr)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add curly braces? That style is not followed everywhere, but we are trying to add curly braces whenever we can.

@cedrik-fuoco-adsk

Copy link
Copy Markdown
Contributor

Thank you @pjurkas! Could you run the pre-commit hook and fix the DCO issue?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Crash in RvApplication::newSessionFromFiles while multiple displays are connected.

2 participants