[GRASS-dev] gis.m crashes on zoom-out

GRASS GIS GRASS GIS developer
1

Hi,

found a bug in gis.m zoom out tool.

start gis.m, select zoom out, draw a box, and poof! gis.m crashes.

happens always in a lat long location when you zoom out past 90NS 180EW
view or a bit harder to trigger in spearfish, but crashes on the 4th or
5th zoom out when the rows*cols gets to be something silly like 500e6*500e6.

presumably g.region exits with an error which isn't handled well.

testing with a few revisions of mapcanvas.tcl:
(the oldest error handling gives the most useful debug info here)

mapcanvas.tcl rev <= 1.57:

popup window:
ERROR: <n=541> ** invalid input **
ERROR: <n=541> ** invalid input **
    while executing
"close $input"
    (procedure "MapCanvas::runprograms" line 40)
    invoked from within
"MapCanvas::runprograms $mon [expr {$mymodified != 0}]"
    (procedure "MapCanvas::drawmap" line 38)
[...]

mapcanvas.tcl rev 1.58, .59:

popup window:
child process exited abnormally
child process exited abnormally
    while executing
"close $input"
    (procedure "MapCanvas::runprograms" line 40)
    invoked from within
"MapCanvas::runprograms $mon [expr {$mymodified != 0}]"
    (procedure "MapCanvas::drawmap" line 38)
[...]

mapcanvas.tcl rev 1.60 and newer:

gis.m crashes completely with "child process exited abnormally" printed
at the terminal prompt.

6.2.1 spearfish + huge region gives this error:
ERROR: Invalid region: North must be larger than South
ERROR: Invalid region: North must be larger than South
    while executing
"close $input"
    (procedure "MapCanvas::runprograms" line 39)
    invoked from within
"MapCanvas::runprograms $mon [expr {$mymodified != 0}]"
    (procedure "MapCanvas::drawmap" line 38)

6.2.1 lat/lon + beyond 90NS gives this error:
ERROR: <n=491.58331367> ** invalid input **
ERROR: <n=491.58331367> ** invalid input **
    while executing
"close $input"
    (procedure "MapCanvas::runprograms" line 39)
    invoked from within
"MapCanvas::runprograms $mon [expr {$mymodified != 0}]"
    (procedure "MapCanvas::drawmap" line 38)

Hamish
(away for the next week+)

2

Hamish wrote:

found a bug in gis.m zoom out tool.

start gis.m, select zoom out, draw a box, and poof! gis.m crashes.

happens always in a lat long location when you zoom out past 90NS 180EW
view or a bit harder to trigger in spearfish, but crashes on the 4th or
5th zoom out when the rows*cols gets to be something silly like 500e6*500e6.

presumably g.region exits with an error which isn't handled well.

Yep.

If you spawn a child process with "open |...", any errors are reported
by way of the corresponding "close" throwing an exception. Tcl's
definition of "error" includes anything being written to stderr (so
any warnings are treated as errors), as well as a non-zero exit code.

Any calls to "close" on a subprocess pipe should be enclosed in a
catch statement.

--
Glynn Clements <glynn@gclements.plus.com>

3

Hamish wrote:
> found a bug in gis.m zoom out tool.
>
> start gis.m, select zoom out, draw a box, and poof! gis.m crashes.
>
> happens always in a lat long location when you zoom out past 90NS
> 180EW view or a bit harder to trigger in spearfish, but crashes on
> the 4th or 5th zoom out when the rows*cols gets to be something
> silly like 500e6*500e6.
>
>
> presumably g.region exits with an error which isn't handled well.

Glynn Clements wrote:

Yep.

If you spawn a child process with "open |...", any errors are reported
by way of the corresponding "close" throwing an exception. Tcl's
definition of "error" includes anything being written to stderr (so
any warnings are treated as errors), as well as a non-zero exit code.

Any calls to "close" on a subprocess pipe should be enclosed in a
catch statement.

gui/tcltk/gis.m$ grep close * | grep -v catch

finds this one in georect.tcl
  proc GRMap::zoom_gregion { args} {

and in histogram.tcl:
  proc GmHist::display { node mod } {

and in rastnums.tcl:
  proc GmRnums::display { node mod } {

and in runandoutput.tcl:
  proc guarantee_xmon {} {

..maybe more?

I don't see the open|g.region in mapcanvas.tcl which triggers this bug?

I seem to remember the georect tool crashing gis.m had been reported.

No time right now to actually fix these myself.

Hamish

4

Hamish wrote:

> Hamish wrote:
> > found a bug in gis.m zoom out tool.
> >
> > start gis.m, select zoom out, draw a box, and poof! gis.m crashes.
> >
> > happens always in a lat long location when you zoom out past 90NS
> > 180EW view or a bit harder to trigger in spearfish, but crashes on
> > the 4th or 5th zoom out when the rows*cols gets to be something
> > silly like 500e6*500e6.
> >
> >
> > presumably g.region exits with an error which isn't handled well.

Glynn Clements wrote:
> Yep.
>
> If you spawn a child process with "open |...", any errors are reported
> by way of the corresponding "close" throwing an exception. Tcl's
> definition of "error" includes anything being written to stderr (so
> any warnings are treated as errors), as well as a non-zero exit code.
>
> Any calls to "close" on a subprocess pipe should be enclosed in a
> catch statement.

gui/tcltk/gis.m$ grep close * | grep -v catch

finds this one in georect.tcl
  proc GRMap::zoom_gregion { args} {

and in histogram.tcl:
  proc GmHist::display { node mod } {

and in rastnums.tcl:
  proc GmRnums::display { node mod } {

and in runandoutput.tcl:
  proc guarantee_xmon {} {

..maybe more?

I don't see the open|g.region in mapcanvas.tcl which triggers this bug?

Line 585, in Mapcanvas::runprograms:

    if {[catch {close $input} error]} {
      puts $error
      exit 1
    }

It catches the error, then prints the error message and exits. Not
really much point in catching it.

--
Glynn Clements <glynn@gclements.plus.com>

5

> > Hamish wrote:
> > > found a bug in gis.m zoom out tool.
> > >
> > > start gis.m, select zoom out, draw a box, and poof! gis.m
> > > crashes.
> > >
> > > happens always in a lat long location when you zoom out past
> > > 90NS 180EW view or a bit harder to trigger in spearfish, but
> > > crashes on the 4th or 5th zoom out when the rows*cols gets to be
> > > something silly like 500e6*500e6.
> > >
> > > presumably g.region exits with an error which isn't handled
> > > well.
>
> Glynn Clements wrote:
> > Yep.
> >
> > If you spawn a child process with "open |...", any errors are
> > reported by way of the corresponding "close" throwing an
> > exception. Tcl's definition of "error" includes anything being
> > written to stderr (so any warnings are treated as errors), as well
> > as a non-zero exit code.
> >
> > Any calls to "close" on a subprocess pipe should be enclosed in a
> > catch statement.
> >

Hamish:

> gui/tcltk/gis.m$ grep close * | grep -v catch
>
>
> finds this one in georect.tcl
> proc GRMap::zoom_gregion { args} {
>
> and in histogram.tcl:
> proc GmHist::display { node mod } {
>
> and in rastnums.tcl:
> proc GmRnums::display { node mod } {
>
> and in runandoutput.tcl:
> proc guarantee_xmon {} {
>
>
> ..maybe more?
>
> I don't see the open|g.region in mapcanvas.tcl which triggers this
> bug?
>

Glynn:

Line 585, in Mapcanvas::runprograms:

    if {[catch {close $input} error]} {
      puts $error
      exit 1
    }

It catches the error, then prints the error message and exits. Not
really much point in catching it.

So what would the soltution look like? Would the above catch+puts+exit
in the histogram or georect tool only bring down those tools and not all
of gis.m as they do now?

histogram.tcl and rastnums.tcl probably need a catch on the open as well?

tcl is mostly beyond me, so I can't really do much to fix these...

Hamish

6

Hamish wrote:

> > > Hamish wrote:
> > > > found a bug in gis.m zoom out tool.
> > > >
> > > > start gis.m, select zoom out, draw a box, and poof! gis.m
> > > > crashes.
> > > >
> > > > happens always in a lat long location when you zoom out past
> > > > 90NS 180EW view or a bit harder to trigger in spearfish, but
> > > > crashes on the 4th or 5th zoom out when the rows*cols gets to be
> > > > something silly like 500e6*500e6.
> > > >
> > > > presumably g.region exits with an error which isn't handled
> > > > well.
> >
> > Glynn Clements wrote:
> > > Yep.
> > >
> > > If you spawn a child process with "open |...", any errors are
> > > reported by way of the corresponding "close" throwing an
> > > exception. Tcl's definition of "error" includes anything being
> > > written to stderr (so any warnings are treated as errors), as well
> > > as a non-zero exit code.
> > >
> > > Any calls to "close" on a subprocess pipe should be enclosed in a
> > > catch statement.
> > >
Hamish:
> > gui/tcltk/gis.m$ grep close * | grep -v catch
> >
> >
> > finds this one in georect.tcl
> > proc GRMap::zoom_gregion { args} {
> >
> > and in histogram.tcl:
> > proc GmHist::display { node mod } {
> >
> > and in rastnums.tcl:
> > proc GmRnums::display { node mod } {
> >
> > and in runandoutput.tcl:
> > proc guarantee_xmon {} {
> >
> >
> > ..maybe more?
> >
> > I don't see the open|g.region in mapcanvas.tcl which triggers this
> > bug?
> >
Glynn:
> Line 585, in Mapcanvas::runprograms:
>
> if {[catch {close $input} error]} {
> puts $error
> exit 1
> }
>
> It catches the error, then prints the error message and exits. Not
> really much point in catching it.

So what would the soltution look like?

Catch then don't exit, just return from the procedure (possibly after
some form of error recovery).

Would the above catch+puts+exit
in the histogram or georect tool only bring down those tools and not all
of gis.m as they do now?

No, "exit" terminates wish, i.e. gis.m.

histogram.tcl and rastnums.tcl probably need a catch on the open as well?

Ideally.

--
Glynn Clements <glynn@gclements.plus.com>

7

I'm going to try to add the needed catch statements discussed below, but
have a question. In the the mapcanvas.tcl example given below, there is an
"exit 1". I don't think I'm the one who added this. When I've done catch
statements, I've had a way to trap and print the error, but haven't found it
necessary to shut down the script. Any reason why I *shouldn't* take this
out and *not* put "exit 1" lines in similar catch clauses for close
statements?

Michael

On 4/24/07 3:09 AM, "Hamish" <hamish_nospam@yahoo.com> wrote:

Hamish wrote:

found a bug in gis.m zoom out tool.

start gis.m, select zoom out, draw a box, and poof! gis.m
crashes.

happens always in a lat long location when you zoom out past
90NS 180EW view or a bit harder to trigger in spearfish, but
crashes on the 4th or 5th zoom out when the rows*cols gets to be
something silly like 500e6*500e6.

presumably g.region exits with an error which isn't handled
well.

Glynn Clements wrote:

Yep.

If you spawn a child process with "open |...", any errors are
reported by way of the corresponding "close" throwing an
exception. Tcl's definition of "error" includes anything being
written to stderr (so any warnings are treated as errors), as well
as a non-zero exit code.

Any calls to "close" on a subprocess pipe should be enclosed in a
catch statement.

Hamish:

gui/tcltk/gis.m$ grep close * | grep -v catch

finds this one in georect.tcl
  proc GRMap::zoom_gregion { args} {

and in histogram.tcl:
  proc GmHist::display { node mod } {

and in rastnums.tcl:
  proc GmRnums::display { node mod } {

and in runandoutput.tcl:
  proc guarantee_xmon {} {

..maybe more?

I don't see the open|g.region in mapcanvas.tcl which triggers this
bug?

Glynn:

Line 585, in Mapcanvas::runprograms:

if {[catch {close $input} error]} {
puts $error
exit 1
}

It catches the error, then prints the error message and exits. Not
really much point in catching it.

So what would the soltution look like? Would the above catch+puts+exit
in the histogram or georect tool only bring down those tools and not all
of gis.m as they do now?

histogram.tcl and rastnums.tcl probably need a catch on the open as well?

tcl is mostly beyond me, so I can't really do much to fix these...

Hamish

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

8

There is a lot of open/close syntax in georect.tcl *without* catch
statements for error trapping.

Generally, I've added these only where there is a likelihood of a problem.
Is it a good idea to add catch to *all* open and close statements? to *all*
close statements, regardless of whether the open statement has a catch? only
to ones likely to cause a crash?

Michael

On 4/24/07 3:09 AM, "Hamish" <hamish_nospam@yahoo.com> wrote:

Hamish wrote:

found a bug in gis.m zoom out tool.

start gis.m, select zoom out, draw a box, and poof! gis.m
crashes.

happens always in a lat long location when you zoom out past
90NS 180EW view or a bit harder to trigger in spearfish, but
crashes on the 4th or 5th zoom out when the rows*cols gets to be
something silly like 500e6*500e6.

presumably g.region exits with an error which isn't handled
well.

Glynn Clements wrote:

Yep.

If you spawn a child process with "open |...", any errors are
reported by way of the corresponding "close" throwing an
exception. Tcl's definition of "error" includes anything being
written to stderr (so any warnings are treated as errors), as well
as a non-zero exit code.

Any calls to "close" on a subprocess pipe should be enclosed in a
catch statement.

Hamish:

gui/tcltk/gis.m$ grep close * | grep -v catch

finds this one in georect.tcl
  proc GRMap::zoom_gregion { args} {

and in histogram.tcl:
  proc GmHist::display { node mod } {

and in rastnums.tcl:
  proc GmRnums::display { node mod } {

and in runandoutput.tcl:
  proc guarantee_xmon {} {

..maybe more?

I don't see the open|g.region in mapcanvas.tcl which triggers this
bug?

Glynn:

Line 585, in Mapcanvas::runprograms:

if {[catch {close $input} error]} {
puts $error
exit 1
}

It catches the error, then prints the error message and exits. Not
really much point in catching it.

So what would the soltution look like? Would the above catch+puts+exit
in the histogram or georect tool only bring down those tools and not all
of gis.m as they do now?

histogram.tcl and rastnums.tcl probably need a catch on the open as well?

tcl is mostly beyond me, so I can't really do much to fix these...

Hamish

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

9

Michael Barton wrote:

There is a lot of open/close syntax in georect.tcl *without* catch
statements for error trapping.

Generally, I've added these only where there is a likelihood of a problem.
Is it a good idea to add catch to *all* open and close statements? to *all*
close statements, regardless of whether the open statement has a catch? only
to ones likely to cause a crash?

It's preferable to add a catch to all open and close statements. Or,
at least, to catch the error somewhere; you could could place a catch
around the entire open/read/close operation.

AFAICT, the only likely reason for the open to fail is if the
executable doesn't exist. Any errors which occur after that (including
problems with shared libraries) will be reported by the corresponding
close statement.

The only case where an error running an individual command should
terminate gis.m itself is if gis.m can't reasonably continue to
operate. Problems with retrieving the current region using g.region
might fall into this category.

--
Glynn Clements <glynn@gclements.plus.com>