Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill recovery not working #190

Open
cajus opened this issue Feb 17, 2024 · 11 comments
Open

Kill recovery not working #190

cajus opened this issue Feb 17, 2024 · 11 comments

Comments

@cajus
Copy link

cajus commented Feb 17, 2024

Hi, just found your lib and started playing with it. One problem I've noticed here is that stopping a simple application via QtCreators "stop" button or pressing Ctrl+c makes the application unstartable with no obvious way to recover. It's always secondary, even though there's no process running anymore.

As the documentation states

Additionally the library can recover from being forcefully killed on *nix systems and will reset the memory block given that there are no other instances running.

, I'm wondering if there's something else needs to be done in main.cpp to make it play happily.

Using master + Qt 6.6.2.

@itay-grudev
Copy link
Owner

What OS was this on?

@cajus
Copy link
Author

cajus commented Feb 17, 2024

Oh. Sorry, forgot that info. Linux. Fedora (Silverblue inside a distrobox) 39.

Update: but it's reproducible without a distrobox - so just take the Fedora 39.

@itay-grudev
Copy link
Owner

itay-grudev commented Feb 17, 2024

@cajus There are significant problems with QSharedMemory and QSystemSemaphore in Qt 6.6. They changed the implementation for the worse. It may not be possible to mitigate the issue on our side.

I am working on a new implementation that doesn't use QSharedMemory but it's not finished yet.

As a workaround you may try to delete it manually using ipcrm. I'm sorry I can't be more helpful for the time being.

@cajus
Copy link
Author

cajus commented Feb 17, 2024

@itay-grudev - ak, ok. Thanks for the info!

@cajus
Copy link
Author

cajus commented Feb 17, 2024

Looks like the linphone guys worked around it by extending your code using DBus on linux for the time being: https://github.com/BelledonneCommunications/linphone-desktop/tree/master/linphone-app/src/app/single-application

@itay-grudev
Copy link
Owner

@cajus That is indeed a clever way for implementing it in Linux, but I can't easily adapt it to Windows.

My WIP implementation right now is using QLocalSockets, where one instance becomes a QLocalServer and becomes the primary. As long as something is listening on that socket - they become a primary and every other instance a secondary.

It looks promising so far, but I'm struggling to find the time to finish it.

@hgkamath
Copy link

hgkamath commented Feb 27, 2024

firstly, many thanks to developer/contributors.
It is believed that the down stream bug, dail8859/NotepadNext#514 is related to this.

Some questions to consider. (re-included here from that issue description)
Q1) should the app, when run as root user, check to see if the desktop belongs to a user and create the shm-file with ownership of user, or maybe not?
Q2) should the app give a different filename to the shm-file per-User-per-Desktop in order to prevent name collision?
Q3) should the app give a better error message to inform the user as to how to correct the problem?
Q4) should the app delete the shm-file after the last user-process using it exits?
Q5) should the shm-file's filename have the string with the app's name in it, in order to clue the user that the file was created by the app.

@itay-grudev
Copy link
Owner

I'll bump it up in my TODO list and see if I can allocate several hours to finish it.

@Ri0n
Copy link

Ri0n commented May 20, 2024

Hi @itay-grudev
Maybe you can push your WIP into a branch?
So those who have more spare time can propose PRs finishing the feature.

@itay-grudev
Copy link
Owner

@Ri0n That is the least I can do. I'll crate a draft PR with all of my outstanding changes.

@itay-grudev
Copy link
Owner

@Ri0n I spent my entire day today hunting down my old code, tidying it up, making sure that it at least compiles. All of my WIP work is in: #192.

The basic functionality works and I've left some implementation notes there. It should be a solid starting point.

manyoso added a commit to nomic-ai/SingleApplication that referenced this issue Aug 29, 2024
This guards against the case of stale shared memory that can happen when
the primary application crashes or is forcefully killed.

Issue itay-grudev#190

Signed-off-by: Adam Treat <treat.adam@gmail.com>
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

No branches or pull requests

4 participants