wxWidgets / wxWidgets

Cross-Platform C++ GUI Library
https://www.wxwidgets.org/
6.1k stars 1.76k forks source link

wxXmlDocument save is not thread safe #15075

Open wxtrac opened 11 years ago

wxtrac commented 11 years ago

Issue migrated from trac ticket # 15075

component: base | priority: low | keywords: xml thread safe

2013-03-01 13:45:40: aimo (Aimo Heikkinen) created the issue


Problem is that save search custom encoding. This search use singleton wxConfig::Get() and change it's path. This path change is only problem.

Callpath: wxRegConfig::SetPath(const wxString & strPath={...}) Line 384 wxFontMapperBase::NonInteractiveCharsetToEncoding(const wxString & charset={...}) Line 616 wxFontMapper::CharsetToEncoding(const wxString & charset={...}, bool interactive=false) Line 183 wxCharsetToCodepage(const char name) Line 1667 wxMBConv_win32::wxMBConv_win32(const char name) Line 2562 wxCSConv::DoCreate() Line 3189 wxCSConv::wxCSConv(const wxString & charset={...}) Line 3022 wxXmlDocument::Save(wxOutputStream & stream={...}, int indentstep=2) Line 1113

I did remove this search from wxFontMapperBase::NonInteractiveCharsetToEncoding and after that it's thread safe (#if 0//wxUSE_CONFIG && wxUSE_FILECONFIG).

Attached test code to repeat this problem. I'm not sure which is the best way to fix this.

wxtrac commented 11 years ago

2013-03-01 13:47:23: aimo (Aimo Heikkinen) uploaded file wxXmlCrashTest.cpp (0.8 KiB)

Test code to repeat this problem.

wxtrac commented 11 years ago

2013-03-01 13:58:25: @vadz changed priority from normal to low

2013-03-01 13:58:25: @vadz changed status from new to confirmed

2013-03-01 13:58:25: @vadz commented

We make no guarantees about thread-safety of these classes so while I agree that it can be unexpected that they're not MT-safe it's not really a surprise and fixing this would be rather non trivial...

wxtrac commented 11 years ago

2013-03-25 13:58:21: aimo (Aimo Heikkinen) commented


I have solution to fix this problem. It doesn't fix the main problem, but I don't think that it do any harm either.

wxtrac commented 11 years ago

2013-03-25 13:59:36: aimo (Aimo Heikkinen) uploaded file fmapbaseConfigProtection.patch (3.0 KiB)

Thread protection

wxtrac commented 11 years ago

2013-03-25 14:13:28: @vadz commented


I'm still not sure about the right way to do this but it seems to me that it can be very unexpected for a simple wxString ctor to block because some other thread has a got a wxFontMappper lock...