[GRASS-dev] suggested temporary feature freeze for wxGUI

[Glynn Clements]

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>