Skip to content

fix: lockup if first run message is up and a client connects - #8601

Merged
nbolton merged 1 commit into
masterfrom
fixCoreProcessSignals
May 20, 2025
Merged

fix: lockup if first run message is up and a client connects#8601
nbolton merged 1 commit into
masterfrom
fixCoreProcessSignals

Conversation

@sithlord48

@sithlord48 sithlord48 commented May 15, 2025

Copy link
Copy Markdown
Member

Fix a possible crash if the first server run dialog is up when a new client and the fingerprint is dealt with and screen added the app would freeze w/ the first server run dialog showing.. (this allows you to close it)

shown in #8599

  1. Have working config (server with clients connected)
  2. Clear settings (on the server only let the client continue to connect)
  3. Start server (do not touch the first time server message box)
  4. See 'Server is running' and fingerprint dialogs at the same time (fingerprint dialog appears in front and no freeze)
  5. Accept the fingerprints and add the screen to the server config (you must fully connect the client, accept it etc)
  6. If the new server dialog is open still the app will freeze.

@sithlord48
sithlord48 requested a review from nbolton May 15, 2025 23:23
@nbolton

nbolton commented May 16, 2025

Copy link
Copy Markdown
Member

How do I reproduce the bug that this fixes? What are the steps? I solved #8599 with my PR.

On master:

  1. Have working config (server with clients connected)
  2. Clear settings
  3. Start server
  4. See 'Server is running' and fingerprint dialogs at the same time (fingerprint dialog appears in front and no freeze)

I was unable to get the GUI to freeze on master. I guess if it's a race condition then maybe I'm just not being unlucky enough. Have you seen a GUI freeze since my PR landed?

My rationale: I don't want to land a "just in case" PR unless I can prove it solves a bug.

@sithlord48

sithlord48 commented May 16, 2025

Copy link
Copy Markdown
Member Author

added steps above

@sithlord48 sithlord48 changed the title fix: race case if first run message is up and a client connects fix: lockup if first run message is up and a client connects May 16, 2025
@nbolton

nbolton commented May 19, 2025

Copy link
Copy Markdown
Member

6. If the new server dialog is open still the app will freeze.

I don't see this freeze. Instead, I see:

image

The 'New client' dialog in front of the 'Server is running' dialog.

Please send me a screenshot of the crash on latest master so I know what I'm looking for.

@sithlord48
sithlord48 force-pushed the fixCoreProcessSignals branch from 5fffcd8 to 1b710c4 Compare May 19, 2025 20:36
@sithlord48

Copy link
Copy Markdown
Member Author

you need to accept the client dialog and add the screen to see the freeze.

@nbolton

nbolton commented May 19, 2025

Copy link
Copy Markdown
Member

you need to accept the client dialog and add the screen to see the freeze.

Still not seeing it. I left the 'Server is running' dialog there, added the client to the grid, and all I see now is...

image

@nbolton

nbolton commented May 19, 2025

Copy link
Copy Markdown
Member

Are you sure you're still able to repro this on master?

@sithlord48
sithlord48 force-pushed the fixCoreProcessSignals branch from 1b710c4 to 73c721c Compare May 19, 2025 20:59
@sithlord48
sithlord48 force-pushed the fixCoreProcessSignals branch from 73c721c to 17f0049 Compare May 19, 2025 21:54
@sithlord48

sithlord48 commented May 19, 2025

Copy link
Copy Markdown
Member Author

100% of the time ( video below is current master )

Deskflow: 1.21.2.240 (34f61869)
Qt: 6.9.0
System: Arch Linux
Session: KDE (wayland)
2025-05-19.18-05-30.mp4

@nbolton

nbolton commented May 20, 2025

Copy link
Copy Markdown
Member

Thanks for the video.

I don't know what I missed yesterday, but yeah, managed to repro. Finally!

Edit: I also cleared the client settings. Perhaps that made the difference.

image

Deskflow: 1.21.2.240 (34f61869)
Qt: 6.8.2
System: Fedora Linux 41 (Workstation Edition)
Session: GNOME (wayland)

@nbolton
nbolton merged commit a5e5bbc into master May 20, 2025
@nbolton
nbolton deleted the fixCoreProcessSignals branch May 20, 2025 11:12
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.

2 participants