msFreeHashItems should not set an error for a table with no items

[cph]

msFreeHashItems is setting an error that eventually gets thrown as a exception in MapScript.

​http://trac.osgeo.org/mapserver/ticket/2229#comment:4

[cph]

this is also a problem with "layer->metadata" in freeLayer

[sdlime]

I don't see the problem. Neither freeLayer nor msFreeMap try to free the hashTableObj. They free the contents of the hash (e.g. msFreeHashItems) as expected.

Am I missing something? Are there errors popping up that can be attributed to this?

Steve

[hobu]

Steve,

I think the problem is related to the setting of this error: ​http://trac.osgeo.org/mapserver/browser/trunk/mapserver/maphash.c#L115

If we have no items, there's nothing to free, but why is this an error?

Howard

Here's a current failure example:

======================================================================
ERROR: MapLayersTestCase.testMapInsertLayer: test insertion of a new layer at default (last) index
----------------------------------------------------------------------
Traceback (most recent call last):
  File "maptest.py", line 100, in testMapInsertLayer
    index = self.map.insertLayer(layer)
  File "/Users/hobu/svn/mapserver/mapscript/python/mapscript.py", line 776, in insertLayer
    def insertLayer(*args): return _mapscript.mapObj_insertLayer(*args)
MapServerError: msFreeHashItems(): Hash table error. No items allocated.

[sdlime]

I was under the impression the initializing a table hash always allocated at least some hash items so that when the error is thrown it's truly an error. It's supposed to mean that you're freeing a hash that wasn't initialized. So you're telling me that can happen?

Steve

[sdlime]

Looks like #2230 is also related to this.

Steve

[hobu]

The default constructor of layerObj for MapScript doesn't initHashTable... should it?

​http://trac.osgeo.org/mapserver/browser/trunk/mapserver/mapscript/swiginc/layer.i#L36

[sdlime]

It does initLayer which in turn should init the hash table. Will have to check that...

Steve

[hobu]

You can provoke it with this. Tracing through gdb, it seems related to the initHashTable of the layer->metadata, but I don't immediately see why.

import mapscript
map = mapscript.mapObj()
layer =mapscript.layerObj()
map.insertLayer(layer)

[hobu]

gdb backtrace:

Breakpoint 1, msFreeHashItems (table=0x3186ec) at maphash.c:101
101         if (table) {
(gdb) bt
#0  msFreeHashItems (table=0x3186ec) at maphash.c:101
#1  0x0108a6af in freeLayer (layer=0x318540) at mapfile.c:2496
#2  0x0109ea36 in msInsertLayer (map=0x1842400, layer=0x318040, nIndex=-1) at mapobject.c:452
#3  0x01065c82 in _wrap_mapObj_insertLayer (self=0x0, args=0x7a800) at mapscript_wrap.c:3782
#4  0x97eb67fe in PyObject_Call ()
#5  0x97f07450 in _PyEval_SliceIndex ()
#6  0x97f08a98 in PyEval_EvalCodeEx ()
#7  0x97f06f6d in _PyEval_SliceIndex ()
#8  0x97f08a98 in PyEval_EvalCodeEx ()
#9  0x97f08bbb in PyEval_EvalCode ()
#10 0x97f21dc5 in PyErr_Display ()
#11 0x97f2395a in PyRun_SimpleFileExFlags ()
#12 0x97f2b42b in Py_Main ()
#13 0x00001f8e in start ()
(gdb) s
102             if(table->items) {
(gdb) s
116               msSetError(MS_HASHERR, "No items allocated.", "msFreeHashItems()");
(gdb) s

[hobu]

Why does initLayer call free layer? Some interaction here is clobbering the layer->metadata item...

(gdb) fin
Run till exit from #0  initHashTable (table=0x3181ec) at maphash.c:65
initLayer (layer=0x318040, map=0x0) at mapfile.c:2424
2424      layer->dump = MS_FALSE;
Value returned is $16 = 0
(gdb) l
2419      layer->requires = layer->labelrequires = NULL;
2420
2421      //layer->metadata = NULL; 
2422      initHashTable(&(layer->metadata));
2423      
2424      layer->dump = MS_FALSE;
2425
2426      layer->styleitem = NULL;
2427      layer->styleitemindex = -1;
2428
(gdb) p &(layer->metadata)
$17 = (hashTableObj *) 0x3181ec
(gdb) p layer->metadata
$18 = {
  items = 0x318290, 
  numitems = 0
}
(gdb) p layer->metadata.items
$19 = (struct hashObj **) 0x318290
(gdb) c
Continuing.

Breakpoint 2, msFreeHashItems (table=0x31872c) at maphash.c:101
101         if (table) {
(gdb) p table
$20 = (hashTableObj *) 0x31872c
(gdb) p *table
$21 = {
  items = 0x0, 
  numitems = 0
}
(gdb) bt
#0  msFreeHashItems (table=0x31872c) at maphash.c:101
#1  0x0108a6af in freeLayer (layer=0x318580) at mapfile.c:2496
#2  0x0109ea36 in msInsertLayer (map=0x1842400, layer=0x318040, nIndex=-1) at mapobject.c:452
#3  0x01065c82 in _wrap_mapObj_insertLayer (self=0x0, args=0x7a800) at mapscript_wrap.c:3782
#4  0x97eb67fe in PyObject_Call ()
#5  0x97f07450 in _PyEval_SliceIndex ()
#6  0x97f08a98 in PyEval_EvalCodeEx ()
#7  0x97f06f6d in _PyEval_SliceIndex ()
#8  0x97f08a98 in PyEval_EvalCodeEx ()
#9  0x97f08bbb in PyEval_EvalCode ()
#10 0x97f21dc5 in PyErr_Display ()
#11 0x97f2395a in PyRun_SimpleFileExFlags ()
#12 0x97f2b42b in Py_Main ()
#13 0x00001f8e in start ()

[hobu]

I think we have it fixed in r6585 now.