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

feat: Add function numberOfInstances() #90

Closed
wants to merge 2 commits into from

Conversation

ouuan
Copy link

@ouuan ouuan commented Jan 29, 2020

This function returns the number of running instances, by adding a new variable in InstancesInfo.

This closes #89.

This function returns the number of running instances, by adding a new variable in InstancesInfo.

This closes itay-grudev#89.
singleapplication.cpp Show resolved Hide resolved
@ouuan ouuan requested a review from itay-grudev January 30, 2020 15:04
ouuan added a commit to ouuan/cpeditor that referenced this pull request Jan 31, 2020
It's easy to use Github Actions to test on multiple OSs and multiple
versions of Qt, so use it instead of travis.ci.

SingleApplication submodule URL is also modified in this commit, please
change it back after itay-grudev/SingleApplication#90 is merged.
ouuan added a commit to ouuan/cpeditor that referenced this pull request Jan 31, 2020
It's easy to use Github Actions to test on multiple OSs and multiple
versions of Qt, so use it instead of travis.ci.

SingleApplication submodule URL is also modified in this commit, please
change it back after itay-grudev/SingleApplication#90 is merged.
ouuan added a commit to ouuan/cpeditor that referenced this pull request Jan 31, 2020
It's easy to use Github Actions to test on multiple OSs and multiple
versions of Qt, so use it instead of travis.ci.

SingleApplication submodule URL is also modified in this commit, please
change it back after itay-grudev/SingleApplication#90 is merged.
ouuan added a commit to ouuan/cpeditor that referenced this pull request Jan 31, 2020
It's easy to use Github Actions to test on multiple OSs and multiple
versions of Qt, so use it instead of travis.ci.

SingleApplication submodule URL is also modified in this commit, please
change it back after itay-grudev/SingleApplication#90 is merged.
@ouuan
Copy link
Author

ouuan commented Jan 31, 2020

I met a problem: The destructor is not called after ::exit() is called, which is called in QCommandLineParser when the option contains --help, --version or invalid options, thus the count is not updated. I found that the destructors of variables declared with static will be called after ::exit(), but that's not recommended for QApplication. Do you have any solutions?

@ouuan
Copy link
Author

ouuan commented Jan 31, 2020

What's more, it would fail if one instance crashed.

@ouuan
Copy link
Author

ouuan commented Jan 31, 2020

I have a solution which can deal with QCommandLineParser, but not crash:

Use qAddPostRoutine to add a function which reduces numberOfInstances.

@ouuan
Copy link
Author

ouuan commented Jan 31, 2020

What about use LocalServer to send a message and wait for replies from other instances in 100ms?

@ouuan
Copy link
Author

ouuan commented Jan 31, 2020

I came up with a trick: Since my actual need is to know whether the current instance is the only running instance, call detach() and check if the shared memory is destroyed should work.

@ouuan
Copy link
Author

ouuan commented Jan 31, 2020

I don't know which solution is the best, or if there are better solutions. However, this PR may be useless. Close this if you like, or tell me how to improve it so that it works correctly after a crash.

@itay-grudev
Copy link
Owner

Yhea. An instance crashing is a serious problem I had to deal with. here is not much you can do about it. Also the Qt Q[Core]Application constructors and places where they just call ::exit() internally. You may be correct that number of instances maybe really hard to achieve with exception to pinging other instances to check for activity, which solution I don't really like because it consumes resources.

I'll think about it for a bit and if I can't come up with an alternative I will close it.

@ouuan
Copy link
Author

ouuan commented Feb 1, 2020

Close this in favor of #91

@ouuan ouuan closed this Feb 1, 2020
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.

Add a function which returns the number of running instances
2 participants