From: Brian M. <br...@gr...> - 2008-09-28 04:50:17
|
This message is mostly for Stephane, but I'm sending it to the list in case anyone else has any insight for us. In gramps30 r10796 and trunk r10797 Stephane committed a potential fix for: http://www.gramps-project.org/bugs/view.php?id=2214 In order to make Gramps not "hang" Stephane cleverly created a new thread to update a progress bar while graphviz is busy rendering a graph. I presume he tested this in Linux and it worked. Unfortunately, in windows it causes Gramps to hang indefinitely. The fact is that Gramps is not set up for running multiple threads with GTK. There is a thread here that provides a pretty good explanation: http://www.daa.com.au/pipermail/pygtk/2003-August/005626.html It looks like it will be quite a bit of work to get Gramps set up to properly run multiple threads. It is probably a fluke if it currently works in Linux at all. And it is probably a crash waiting to happen if the timing is ever right. I personally recommend that we revert those commits until we can find a more proper solution. It might even be worth expediting a 3.0.3 release so that Windows users can again use the graphs. I'm open to suggestions. Anybody have any advice for us? Thanks, ~Brian |
From: S. C. <ste...@gm...> - 2008-09-28 05:03:19
|
First thing first: sorry everyone. It was tested on Linux, but I don't use Windows. Next: Wow. That was a fix I made back over 3 months ago -- I'm guessing no-one that uses Windows has used the gramps30 branch since June? BTW, I must state that I disagree with the statement that it is a fluke it works at all in Linux. Here is a description of the fix: 1) when it comes time to call "dot" (graphviz)... 2) create a 2nd thread, which will: 3) create a pulse-style progress bar since we don't know how long it will take 4) create a timer to regularly pulse the progress bar 5) call system.os('dot ...') to create the requested graph 6) destroy the pulse progress bar 7) end-of-thread The change can be seen here: http://gramps.svn.sourceforge.net/viewvc/gramps/branches/gramps30/src/ReportBase/_GraphvizReportDialog.py?r1=10796&r2=10795&pathrev=10796 So...here is a suggestion. For Windows, could we #ifdef out (sorry, my C/C++ bleeding through here) the thread creation and instead call that function as a normal function call? For example (I forget my Python here, bear with me): #if WINDOWS self.generate_output_file_from_dot_file, ('dot -Tps2 -o"%s" "%s"', "application/postscript") #else thread.start_new_thread( self.generate_output_file_from_dot_file, ('dot -Tps2 -o"%s" "%s"', "application/postscript")) #endif Stéphane On Sat, Sep 27, 2008 at 21:23, Brian Matherly <br...@gr...> wrote: > This message is mostly for Stephane, but I'm sending it to the list in case anyone else has any insight for us. > > In gramps30 r10796 and trunk r10797 Stephane committed a potential fix for: > http://www.gramps-project.org/bugs/view.php?id=2214 > > In order to make Gramps not "hang" Stephane cleverly created a new thread to update a progress bar while graphviz is busy rendering a graph. > > I presume he tested this in Linux and it worked. Unfortunately, in windows it causes Gramps to hang indefinitely. > > The fact is that Gramps is not set up for running multiple threads with GTK. There is a thread here that provides a pretty good explanation: > http://www.daa.com.au/pipermail/pygtk/2003-August/005626.html > It looks like it will be quite a bit of work to get Gramps set up to properly run multiple threads. > > It is probably a fluke if it currently works in Linux at all. And it is probably a crash waiting to happen if the timing is ever right. > > I personally recommend that we revert those commits until we can find a more proper solution. It might even be worth expediting a 3.0.3 release so that Windows users can again use the graphs. > > I'm open to suggestions. Anybody have any advice for us? > > Thanks, > > ~Brian > |
From: Brian M. <br...@gr...> - 2008-09-29 02:56:46
Attachments:
graph_progress_thread_1.patch
|
Stephane, > First thing first: sorry everyone. It was tested on Linux, > but I > don't use Windows. It's not a problem. I didn't mean to put you on the spot in front of everyone. I just thought other developers might be interested in this topic. > Wow. That was a fix I made back over 3 months ago -- > I'm guessing > no-one that uses Windows has used the gramps30 branch since > June? I actually saw the problem some time ago, but I figured it was something wrong with my installation, not Gramps. I should have looked into it sooner. > BTW, I must state that I disagree with the statement that > it is a > fluke it works at all in Linux. > > Here is a description of the fix: > > 1) when it comes time to call "dot" (graphviz)... > 2) create a 2nd thread, which will: > 3) create a pulse-style progress bar since we don't > know how long it will take > 4) create a timer to regularly pulse the progress bar > 5) call system.os('dot ...') to create the > requested graph > 6) destroy the pulse progress bar > 7) end-of-thread Your logic is sound, but your implementation is flawed. The error occurs in step #4 above. You are accessing GTK functions from a different thread, but GTK *is not thread safe*. There is a decent explaination here: http://research.operationaldynamics.com/blogs/andrew/software/gnome-desktop/gtk-thread-awareness.html So the fact that somehow your accessing GTK from multiple threads works on Linux is a fluke. Because it shouldn't work. We have two options. Modify Gramps to use GTK in an appropriate multi-threaded way, or only make GTK calls from the main thread. I think modifying Gramps to use GTK properly in a multi-threaded environment will be a lot of work. So I have attached a patch that modifies your concept to keep GTK calls in the main thread. It goes like this: 1) when it comes time to call "dot" (graphviz)... 2) create a pulse-style progress bar since we don't know how long it will take 3) create a 2nd thread, which will call system.os('dot ...') to create the requested graph 4) create a loop in the main thread to regularly pulse the progress bar 5) when the thread ends destroy the pulse progress bar It seems to work well in Windows. Could you please try it in Linux to see that it works properly? The patch is against the latest trunk. > So...here is a suggestion. For Windows, could we #ifdef... I don't think that will be necessary. I'm sure we can figure out a solution. Thanks, ~Brian |
From: S. C. <ste...@gm...> - 2008-09-29 06:21:24
|
>> BTW, I must state that I disagree with the statement that it is a fluke it works at all in Linux. I take it back, and you were absolutely correct; I was lucky this worked. GTK can work with threads, but you have to know what you're doing, especially when it comes to handling the big lock, of which I was ignorant. This description was quite informative and is what finally got me to understand what they meant by "thread aware but not thread safe": GTK+ is "thread aware" but not thread safe — it provides a global lock controlled by gdk_threads_enter()/gdk_threads_leave() which protects all use of GTK+. That is, only one thread can use GTK+ at any given time. Unfortunately the above holds with the X11 backend only. With the Win32 backend, GDK calls should not be attempted from multiple threads at all. (Source: http://library.gnome.org/devel/gdk/stable/gdk-Threads.html#id2978818) So the use of GDK in the secondary thread like I attempted wont work. We could insert the proper enter/leave code necessary for doing the big GDK lock, but this is still only a solution on non-Windows platforms. Your patch, on the other hand: > 1) when it comes time to call "dot" (graphviz)... > 2) create a pulse-style progress bar since we don't know how long it will take > 3) create a 2nd thread, which will call system.os('dot ...') to create the requested graph > 4) create a loop in the main thread to regularly pulse the progress bar > 5) when the thread ends destroy the pulse progress bar > > It seems to work well in Windows. Could you please try it in Linux to see that it works properly? The patch is against the latest trunk. Yes, I applied it tonight on trunk, and it works for me on Ubuntu 8.04. The OS no longer prompts me to kill the application, and the GRAMPS window doesn't go dark because it is unresponsive. I see you generalized the call to create the KITT-style progress bar. If I may suggest: could we change the text in _run_long_rocess_in_thread() so we can display the filename instead of just "Processing File"? That way the user knows (more-or-less) what file we're processing. Stéphane |
From: Brian M. <br...@gr...> - 2008-09-29 12:17:12
|
Stephane, > I see you generalized the call to create the KITT-style > progress bar. > If I may suggest: could we change the text in > _run_long_rocess_in_thread() so we can display the filename > instead of > just "Processing File"? That way the user knows > (more-or-less) what > file we're processing. Sure. I'll add that in and them commit it to trunk and 3.0. ~Brian |
From: S. C. <ste...@gm...> - 2008-09-28 05:07:38
|
> I personally recommend that we revert those commits until we can find a more proper > solution. It might even be worth expediting a 3.0.3 release so that Windows users can > again use the graphs. Does GRAMPS hang on startup, or only when creating one of the Graphviz graphs? If you choose the output format as .dot versus .png/.gif/etc does GRAMPS then run correctly? You could then manually call dot from the Windows command-line to get the graph you want. (Not a solution, but until we get a fix in place you could generate the graph you want without having to revert back to 3.0.1.) Stéphane |