Closed
Bug 99382
Opened 23 years ago
Closed 9 years ago
Charset conversion code is not threadsafe
Categories
(Core :: Internationalization, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla1.2alpha
People
(Reporter: bbaetz, Assigned: jshin1987)
References
Details
(Keywords: assertion, intl)
As part of bug 78148, I want to be able to call nsIFile::GetUnicodeLeafName from
the file transport thread. I also need to be able to use the nsICollation stuff
to do string comparison of file names.
This would be a _lot_ easier for me if I could do this from the file transport
thread (proxying this off to the UI thread would likely be slow). However, I'm
hitting assertions because lots of classes aren't declared with
NS_ISUPPORTS_THREADSAFE_IMPLx.
How threadsafe is this stuff? I can work arround it if it would be lots of work,
but there would be a performance cost.
The occasional calls to setlocale()and friends probably don't help
Comment 2•23 years ago
|
||
I am very very confused now. the summary said Charset Conversion code is not
threadsafe and the body of the bug said the nsICollation is not threadsafe.
Which one is the issue. I don't think charset conversions is not threadsafe.
Reporter | ||
Comment 3•23 years ago
|
||
The following are the assertions I get when using nsIFile::GetUnicodeLeafName
and nsICollation on the file transport thread (I've stripped out the assertions
I also get when not using this code):
###!!! ASSERTION: nsCharsetAlias2 not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsTextToSubURI not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsCaseConversionImp2 not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: HandleCaseConversionShutdown3 not thread-safe: 'owningThread
== NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp,
line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsCaseConversionImp2 not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsBasicDecoderSupport not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: RDFXMLDataSourceImpl not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsUNIXCharset not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsPosixLocale not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsBasicDecoderSupport not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! ASSERTION: nsUnicodeDecodeHelper not thread-safe: 'owningThread ==
NS_CurrentThread()', file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
###!!! Break: at file /home/bbaetz/src/mozilla/xpcom/base/nsDebug.cpp, line 528
If they are threadsafe, they should declare themselves as such. However, the
collation stuff definately isn't threadsafe, due to the use of setlocale.
Comment 4•23 years ago
|
||
The question I have is:
1. if these classes are not thread safe, should we make them thread safe? Why?
2. how can we ensure they are thread safe?
3. what make them not thread safe.
Reporter | ||
Comment 5•23 years ago
|
||
1. Well, if they're not threadsafe, then it means that people can't use them
unless they're on the ui thread. My example is that I cannot get the 'real'
filename of a unicode file from the file transport thread, or encode urls using
nsITextToSubURI, or even get the charset of the native filesystem.
3. I don't know. The calls to setlocale (in the unix stuff, see
nsCollationUnix::Do{Set,Restore}Locale) aren't protected bya lock, so if two
threads are calling collation functions at the same time, then this may produce
the incorrect results. Other platforms may have similar issues, depending on
whether their version of setlocale is per thread or per process.
2. go through the code, and add locks as appropriate, I guess, and then change
the _ISUPPORTS macros to announce that you are threadsafe.
My workarround will involve proxying the calls to the ui thread. This is not
desirable from a perf point of view, but for most cases (including mine) it is
acceptable. This isn't urgent, since I can work arround it. However, if its not
much work then it may be worth fixing. from a quick glance, most of the
functions appear self-contained service functions which will be threadsafe if
the underlying OS functions are, but I don't know this code at all.
Comment 6•23 years ago
|
||
shanjian- can you look at the thread issue when you have time ?
Assignee: ftang → shanjian
Priority: -- → P3
Updated•23 years ago
|
Target Milestone: --- → mozilla0.9.7
Updated•23 years ago
|
Target Milestone: mozilla0.9.7 → mozilla1.2
Comment 8•23 years ago
|
||
We're getting lots of assertions at startup time for these, yet this has target
milestone moz 1.2. Either we need to get this fixed sooner, or we need to get
the assertion turned off. Should there be a new bug to cover turning the
assertion off?
Comment 9•22 years ago
|
||
any progress on this bug? can the target milestone be updated?
this bug has resulted in a number of workarounds inside the necko codebase and
it certainly blocks necko from ever being run on a background thread (i.e.,
because of this bug and a number of other threadsafety bugs, it is not possible
to AsyncOpen a channel on a background thread).
Comment 10•22 years ago
|
||
Switching QA contact from andreasb to bobj. Bob, please re-assign as you see fit.
QA Contact: andreasb → bobj
Comment 11•22 years ago
|
||
Frank and Shanjian, Do you have any more info on this bug?
Comment 12•22 years ago
|
||
i should add that PSM currently loads string bundles on the socket thread. i'm
not sure if that is expected to be OK, but as a result we may enter the charset
conversion code on a background thread.
Assignee | ||
Comment 13•21 years ago
|
||
I was led here thanks to timeless in bug 229032 comment #51
Comment 14•20 years ago
|
||
shanjian is no longer working on mozilla for 2 years and these bugs are still
here. Mark them won't fix. If you want to reopen it, find a good owner first.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → WONTFIX
Comment 16•20 years ago
|
||
Mass Re-opening Bugs Frank Tang Closed on Wensday March 02 for no reason, all
the spam is his fault feel free to tar and feather him
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 17•20 years ago
|
||
Reassigning Franks old bugs to Jungshik Shin for triage - Sorry for spam
Assignee: nobody → jshin1987
Status: REOPENED → NEW
Updated•15 years ago
|
QA Contact: bobj → i18n
This was fixed for off-the-main-thread HTML parsing.
Status: NEW → RESOLVED
Closed: 20 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•