Skip to content

Add a runtime warning about trying to start the same job twice#272

Open
CarlSchwan wants to merge 1 commit into
frankosterfeld:mainfrom
CarlSchwan:work/carl/warn-about-starting-job-twice
Open

Add a runtime warning about trying to start the same job twice#272
CarlSchwan wants to merge 1 commit into
frankosterfeld:mainfrom
CarlSchwan:work/carl/warn-about-starting-job-twice

Conversation

@CarlSchwan

Copy link
Copy Markdown

No description provided.

@CarlSchwan CarlSchwan force-pushed the work/carl/warn-about-starting-job-twice branch from b34a98c to b2a40f4 Compare June 22, 2025 21:41
@CarlSchwan CarlSchwan force-pushed the work/carl/warn-about-starting-job-twice branch from b2a40f4 to dc56c83 Compare June 22, 2025 21:42
Comment thread qtkeychain/keychain.cpp

void Job::start()
{
if (d->started) {

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.

Do we have to take care about multithreaded use? Sorry, I don't know about the general policy on whether Qt Keychain supports multithreaded access or not.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Jobs aren't thread-safe and must be created and accessed in a single thread, so here that's fine. Which of the backends would even support multithreaded access is a good question, I always assumed that people use this from the main/UI thread.

Comment thread qtkeychain/keychain.cpp
void Job::start()
{
if (d->started) {
qCritical() << "Starting the same twice. This is not supported, aborting.";

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'd give some more context, like "Starting the same QtKeychain job twice". Otherwise looks good

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Or maybe just "The QtKeychain job is already started, ignoring.". So it also stays correct for N start() calls

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.

3 participants