-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106242/#review18866
-----------------------------------------------------------


That's pretty bonkers. This class was never meant to be used like that. Holding 
an object in two threads is not going to work unless you make the entire 
communication synchronous effectively reducing all the multi-threading aspects 
to a "very complicated single thread". It just complicates this class.
The proper fix is to, instead of trying to synchronize one object owned by 
multiple threads, make the speller property of just the thread that does the 
spelling (or even just moveToThread it if you have to) and instead of calling 
setLanguage from another thread create a signal/slot combination between the 
parent thread and the speller thread and send a language change request by 
emitting a signal from the parent thread.  

- Zack Rusin


On Sept. 9, 2012, 10:06 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106242/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2012, 10:06 p.m.)
> 
> 
> Review request for kdelibs and Plasma.
> 
> 
> Description
> -------
> 
> Krunner's spellcheck plugin has been pretty broken since 
> bd291d21f096a714a171e7af3a534ba345ca5659 (about two years ago) because it 
> called Sonnet::Speller::setLanguage every time the spellchecker was invoked, 
> which was (very much) not thread-safe.
> 
> This patch makes Sonnet::Speller::setLanguage threadsafe by protecting all 
> access to the internal dict pointer using QReadWriteLock. 
> 
> A related review request is 106244, which adds more fixes to the spellcheck 
> feature.
> 
> 
> This addresses bugs 264779 and 303831.
>     http://bugs.kde.org/show_bug.cgi?id=264779
>     http://bugs.kde.org/show_bug.cgi?id=303831
> 
> 
> Diffs
> -----
> 
>   kdecore/sonnet/speller.cpp b19e74d 
> 
> Diff: http://git.reviewboard.kde.org/r/106242/diff/
> 
> 
> Testing
> -------
> 
> Compiled, installed, used for a week or so, spellchecked a bunch of things.
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

Reply via email to