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

thread safety #41

Closed
bornschein opened this issue Nov 20, 2013 · 8 comments
Closed

thread safety #41

bornschein opened this issue Nov 20, 2013 · 8 comments

Comments

@bornschein
Copy link

Hello,
could it be, that the client code generated by kdwsdl2cpp is not thread safe?
When calling "executeCommand" simultaneously in concurrent threads I get weird crashes. With a single thread It is working like a charm though.

I couldn't find any documentation about the client side thread safety and therefore appreciate any comment on it.
Thank you in advance
Stefan

@dfaure-kdab
Copy link
Member

Do you create a different instance of the generated service object (or of KDSoapClientInterface, if using that directly), in each thread?

Using the same instance isn't threadsafe indeed.

@bornschein
Copy link
Author

Yes, we were instantiating one service object per thread.

However, we might have found the issue:
There seems to be a problem in KDSoapNameSpaceManager. It is initializing static QStrings within function scope, ex

QString KDSoapNamespaceManager::xmlSchema1999()
{
static QString s = QString::fromLatin1("http://www.w3.org/1999/XMLSchema");
return s;
}

When multiple threads come in here the very first time simultaneously, there is a problem. This was actually happening pretty often (90% of the time).

After moving all static variable initializations out of the function bodies, the problem seems to be gone.

Do you want me to commit the changes?

Best regards
Stefan

@dfaure-kdab
Copy link
Member

Ah, good find.
Global static objects are not recommended in libraries though, they create an "order of creation" (and of destruction) issue. A solution for global static objects is Q_GLOBAL_STATIC.

But maybe we can do something simpler here: QStringLiteral() with Qt5 and QString::fromLatin1() with Qt4 (slower, but "fixed" in Qt5).

@bornschein
Copy link
Author

Sure, something like this up front?

namespace
{
const QString s1999 = QString::fromLatin1 ("http://www.w3.org/1999/XMLSchema");
const QString s2001 = QString::fromLatin1 ("http://www.w3.org/2001/XMLSchema");
const QString si1999 = QString::fromLatin1 ("http://www.w3.org/1999/XMLSchema-instance");
const QString si2001 = QString::fromLatin1 ("http://www.w3.org/2001/XMLSchema-instance");
const QString sEnv = QString::fromLatin1 ("http://schemas.xmlsoap.org/soap/envelope/");
const QString s200305 = QString::fromLatin1 ("http://www.w3.org/2003/05/soap-envelope");
const QString sEnc = QString::fromLatin1 ("http://schemas.xmlsoap.org/soap/encoding/");
const QString sEnc200305 = QString::fromLatin1 ("http://www.w3.org/2003/05/soap-encoding");
}

and then simple implementations like this:

QString KDSoapNamespaceManager::xmlSchema1999()
{
return s1999;
}

@dfaure-kdab
Copy link
Member

These are the "global static objects" that I know can give trouble, see my previous message.

@bornschein
Copy link
Author

true, they can give trouble due to order problems. However I am not sure if - in this particular case - there is a danger of "out of sequence" initialization. These strings are independent from each other and just used within this cpp file. So, the order doesn't matter, as long as they are initialized at all.
The alternative would be to create a new QString on the fly, every time the function is invoked. This might be a little bit slower, which most likely can be ignored because this extra time can't compare against the overall network latency anyway.

Your call. It is your library :)
Stefan

@dfaure-kdab
Copy link
Member

The problem is order of initialization not between these strings, but compared to the global statics in Qt itself.

I'll create strings on the fly for Qt4 (thanks for the confirmation that you see no issue with that), and use QStringLiteral for Qt5 (which doesn't need any conversions, so it will be very fast).

@dfaure-kdab
Copy link
Member

Further investigation reveals that gcc implements thread safety for function-local statics (as mandated by C++11, but it has done so for a much longer time), while MSVC doesn't (and still doesn't in MSVC2013).
This explains the behavior difference on Linux and Windows.

I committed the removal of "static" now in 5027b0c, sorry for the delay.

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

2 participants