Thread safety for ID3v1 genres
Stephen F. Booth
me at sbooth.org
Mon May 2 13:27:18 CEST 2011
>> I've been experiencing a crash that I can reproduce 100% of the time,
>> but I'm struggling on coming up with a good fix.
>
> In theory the new atomic ref counting in trunk should solve this. Can you see if you can still reproduce it with trunk?
I should have mentioned this in my original message- I am using taglib
trunk on Mac OS X10.6.7 and gcc 4.2.1.
Perhaps I'm missing something obvious, but as far as I can tell there
is no regard to refcounting in toCString. The entire method body
consists of:
const char *String::toCString(bool unicode) const
{
delete [] d->CString;
std::string buffer = to8Bit(unicode);
d->CString = new char[buffer.size() + 1];
strcpy(d->CString, buffer.c_str());
return d->CString;
}
So the first line of code executed, regardless of the string's
refcount, deletes the old buffer. Thus any string object held by more
than one thread can't be trusted to give a pointer with a guaranteed
lifetime from toCString.
>> static const int genresSize = 148;
>> - static const String genres[] = {
>> + static const char * genres[] = {
>> "Blues",
>
> This is actually one of those things that I've mentioned that if TagLib wants to be truly reentrant that the static initializers (which there are a fair number of) will need to be possible to invoke manually, mutexed or to go away. While I doubt the initialization the problem there, the function is fundamentally not reentrant since there's a possible race condition on the first call to the function.
I still would suggest the patch above, because no C++ objects are
being created at static initialization time.
Stephen
More information about the taglib-devel
mailing list