[mapguide-internals] std::string not thread safe on Linux

[Traian Stanev]

Hi Trevor,

I specifically suggested returning const wchar_t* -- notice the const, in cases where it currently returns CREFSTRING. I did not suggest replacing all STL strings by manual string allocations. The const makes sure that the caller does not attempt to free the string. Doing this (and only this) will not have any lifetime issues, since that const string pointer is owned by its STRING parent, and it will be freed by STL like it is today.

Any code that calls such a function and assigns the result to a STRING today, will not have to change, while gaining speed and thread safety.

Traian


-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Trevor Wekel
Sent: Thursday, August 05, 2010 5:46 PM
To: MapGuide Internals Mail List
Subject: RE: [mapguide-internals] std::string not thread safe on Linux

Hi Traian,

The APIs in question are internal (C++ only) so I could return a const wchar_t*.  However, all of the published MapGuide APIs return STRING (std::wstring).  For the sake of consistency, I chose to return STRING.

We could make a wholesale change of STRING to wchar_t* but then we would have to manually deal with string lifetime, allocation and deallocation.  This would make MapGuide C++ development even more complicated.  I would not veto a change to raw wchar_t* but I would give it a "-0" vote because I believe a thread-safe C++ string class is a much better approach from a maintainability perspective.

I do not like string allocation either.  It is wasteful, useless, and sucks back performance.

On the other hand, design patterns in a multi-threaded open source code base are very important.  Consistent patterns that work (ie. are thread-safe) insulate developers from having to think about thread-safety for every line of code they write.  If we are truly serious about community involvement, I believe we should also consider ease of implementation for individuals that do not have 6+ of experience with the guts of MapGuide.

If we find a thread-safe, reference counted C++ class (ICU UnicodeString?) that avoids allocation I would be all for it.

Regards,
Trevor


-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Traian Stanev
Sent: August 5, 2010 1:32 PM
To: 'MapGuide Internals Mail List'
Subject: RE: [mapguide-internals] std::string not thread safe on Linux


Hi Trevor,

Instead of:

    CREFSTRING function(params);

Is it not better to change to:

    const wchar_t* function(params);

?

In the change you describe (returning STRING) could you still potentially end up making an assignment of string object from one thread to another? I mean the STRING object which you return is constructed by function() and then copied by the calling code in the assignment? I admit I am not exactly clear on where the thread boundary is in this case though.

Regardless of all this, you would be making two string allocations instead of one (one by function() and one by the caller of function), while returning const wchar_t* would mean you are still only allocating one string.


Traian