06-11-2011, 02:56 PM | #31 |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
Working on it...
|
06-11-2011, 03:20 PM | #32 |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
@kovid, back the changes out. There appears to be a situation where the called and calling threads are the same, in which case it cannot work. I will get back to you if/when I find a solution.
|
Advert | |
|
06-11-2011, 03:56 PM | #33 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
reverted.
|
06-11-2011, 04:27 PM | #34 |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
OK, yet again...
It seems that all the done functions with the exception of device_detected are called from the GUI thread. I am not sure how this comes to be, but it is. FunctionDispatcher would lock up in that case, because one thread is to put the request and another thread consume it. If both are the same thread, then the consumer never gets a chance. Using Dispatcher in this case works, but given that the threads are the same the call to the done function will happen *after* everything else that goes on, including signal catching and the like. This is wrong. I have pushed changes where FunctionDispatcher checks if the calling and called thread are the same, and if so directly calls done without signals or queues. I traced it with print statements and it did the right thing when the threads were and were not the same. I also tested it (again) multiple times with my sony and got the behavior I expected. I think it is stable. I suggest that you merge my branch back in and let meme have at it. If it doesn't fix his problems, then back it out (again) and we will need to do more work to see why he is being called out of turn. |
06-11-2011, 05:07 PM | #35 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
I'm not happy about the changes to FunctionDispatcher. You're comparing the thread that calls __call__ with the thread that calls __init__. What you should be doing (I think) is comparing the thread that calls __call__ with the gui thread, as q.put(res) is called in the GUI thread. Use the is_gui_thread() function to do that.
|
Advert | |
|
06-11-2011, 05:14 PM | #36 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
I've changed FD to use is_gui_thread(). Given that the calls to job_done are happening in the GUI thread already, I doubt using FunctionDispatcher will make any difference to meme. Presumably, there is another signal chain earlier in the call sequence.
|
06-11-2011, 05:15 PM | #37 |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
I did that on purpose. The semantics of FunctionDispatcher are that the signal will be run in the thread that created the instance, thus the thread active in __init__. If the calling thread is the same as the creating thread, then the signal will never be received because the thread will be the same. There is no rule that the instantiating thread is the GUI thread.
|
06-11-2011, 05:18 PM | #38 | |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
Not convinced this is correct for the reasons I gave earlier.
Quote:
|
|
06-11-2011, 05:30 PM | #39 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
While I agree that the comment of FunctionDispatcher claims that it calls the function in the thread that it was created in, the situation isn't that clear cut.
From the Qt docs at http://doc.qt.nokia.com/latest/threa...across-threads Queued Connection The slot is invoked when control returns to the event loop of the receiver's thread. The slot is executed in the receiver's thread. That, in this case, means the thread the FD object was created in as the comment claims. However, if the thread does not have an event loop (which is almost always the case for all non GUI threads in calibre), I'm guessing it is actually called in the GUI thread. Perhaps a good solution is to raise an exception in FD.__init__ if the current thread is not the GUI thread. |
06-11-2011, 05:35 PM | #40 | |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
Quote:
I assume you will make the change... EDIT: should Dispatcher make the same check? It clearly is making the same assumption. |
|
06-11-2011, 05:41 PM | #41 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
I've committed the change. Semantically we should add it to Dispatcher as well, but since it isn't critical (in that it causes no deadlocks) I'd rather leave well enough alone.
|
06-11-2011, 05:45 PM | #42 |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
@meme: your turn.
|
06-11-2011, 05:47 PM | #43 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
@meme: Wait.
@chaley: I haven't made the change to use FunctionDispatcher in gui2.device however. Looking at the code, the done function is called from BaseJob.update which in turn is called in the GUI thread by JobsManager._update() So changing those to FD wont make any difference. |
06-11-2011, 05:50 PM | #44 | |
Grand Sorcerer
Posts: 11,740
Karma: 6997045
Join Date: Jan 2010
Location: Notts, England
Device: Kobo Libra 2
|
Quote:
|
|
06-11-2011, 05:54 PM | #45 |
creator of calibre
Posts: 43,852
Karma: 22666666
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
|
It depends on whether JobsManager calls update() before starting the next job or not. I'm trying to figure out whether that is the case.
|
|
Similar Threads | ||||
Thread | Thread Starter | Forum | Replies | Last Post |
eBooks from the library —waiting waiting | Khendron | Kobo Reader | 9 | 05-12-2010 01:02 PM |
Which one of these activities do you dedicate more time to? | daviddem | Lounge | 24 | 12-26-2008 11:40 AM |
iRex to expand B2B activities / speaks of possible IPO | Alexander Turcic | iRex | 12 | 04-10-2007 09:36 AM |