[GRASS-dev] suggested temporary feature freeze for wxGUI

[Michael Barton]

Thanks for the suggestions.

Refactoring is certainly in order. We've started some of this already,  
but there is a lot more to do. I don't think we are overtly using  
multithreading. However, Python may be doing this on its own I suppose.

Michael

Michael

On Sep 7, 2008, at 2:44 PM, Glynn Clements wrote:

>
> Michael Barton wrote:
>
>> Seeing all the recent bug reports coming in for the new wxPython GUI
>> is frustrating...not because of the testing or reports, which are  
>> very
>> good to have, but because almost all of these features worked
>> previously.
>>
>> This is a function of the complexity of the GUI. Unlike command
>> modules, it involves a very large amount of code that is very tightly
>> interlinked in sometimes not very obvious ways--even to those of us
>> who are writing it. A feature or even a bug fix added in one place  
>> can
>> cause a cascade of effects elsewhere and break something that seems
>> completely unrelated.
>
> In which case, it's probably time to start thinking about refactoring
> the code.
>
> E.g. class BufferedWindow in mapdisp.py is ~2500 lines. That includes
> some massive "case" (i.e. if/elif/...) statements in OnLeftUp and
> OnRightUp which should probably be replaced with e.g.
> self.currentTool.OnLeftUp() etc, with a separate class for each tool.
>
>> 2) increasing robusticity. This is trickier, but I'm thinking of
>> situations where a feature works fine under normal circumstances, but
>> can fail or give an error under abnormal circumstances like repeated
>> mouse clicking, odd combinations of actions, etc.
>
> I would guess that most of this is due to the use of threads. Writing
> concurrent code is hard, and performing meaningful testing is all but
> impossible (textbooks on the subject often include extensive sections
> on formal proofs using temporal logic).
>
> [We have seen this issue on a smaller scale with NVIZ calling Tcl's
> "update" during rendering.]
>
> As a general rule, auxilliary threads (i.e. everything except for the
> main event-handling thread) should avoid accessing any shared data. If
> necessary, data should be (deep-) copied within the main thread and
> the copy passed to the auxilliary thread when it is created. Any
> interaction with the rest of the process should be via wx.CallAfter.
>
> If shared data has to be read or written directly by auxilliary
> threads, it needs to be protected against concurrent access with
> locks. Code which performs a read-modify-write cycle on data needs to
> retain the lock for the duration of the cycle.
>
> Also, bear in mind that GRASS itself isn't particularly thread-safe.
> Many library functions aren't thread-safe, and anything which modifies
> fixed files (WIND, CURGROUP, etc) is risky. The safest solution is to
> ensure that you never run multiple GRASS commands concurrently. If
> necessary, just block the main thread until any other threads have
> finished.
>
> -- 
> Glynn Clements <glynn at gclements.plus.com>