FITS matrix example (tutorial) does not work

With recent versions of root (5.32 or svn), the FITS_tutorial1.C crashes on two machines that I tested it on:

....................................
Exposure time = 5.007
....................................
We can read the image as a matrix of values.
This feature is useful to do image processing, e.g:
histogram equalization, custom filtering, ...

292x389 matrix is as follows

     |        0    |        1    |        2    |        3    |        4    |
----------------------------------------------------------------------

 *** Break *** segmentation violation
   0 |
===========================================================
There was a crash (#7 0x0068629d in SigHandler () from /opt/pi/ext/dload/root/lib/libCore.so).
This is the entire stack trace of all threads:
===========================================================
#0  0x00110416 in __kernel_vsyscall ()
#1  0x00225953 in __waitpid_nocancel () from /lib/libc.so.6
#2  0x001c9bab in do_system () from /lib/libc.so.6
#3  0x001878dd in system () from /lib/libpthread.so.0
#4  0x00681d1d in TUnixSystem::Exec ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#5  0x006898dd in TUnixSystem::StackTrace ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#6  0x006861cb in TUnixSystem::DispatchSignals ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#7  0x0068629d in SigHandler () from /opt/pi/ext/dload/root/lib/libCore.so
#8  0x0067f154 in sighandler () from /opt/pi/ext/dload/root/lib/libCore.so
#9  0x006b0b71 in textinput::TerminalConfigUnix::HandleSignal ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#10 0x006b0fd6 in (anonymous namespace)::TerminalConfigUnix__handleSignal ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#11 <signal handler called>
#12 0x019db578 in TMatrixT<double>::operator() ()
   from /opt/pi/ext/dload/root/lib/libMatrix.so
#13 0x01a059ae in TMatrixTBase<double>::Print ()
   from /opt/pi/ext/dload/root/lib/libMatrix.so
#14 0x007c46fa in G__G__Base2_10_0_45 ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#15 0x00c37dca in Cint::G__ExceptionWrapper ()
   from /opt/pi/ext/dload/root/lib/libCint.so
#16 0x00cf2a76 in G__execute_call () from /opt/pi/ext/dload/root/lib/libCint.so
#17 0x00cf702d in G__call_cppfunc () from /opt/pi/ext/dload/root/lib/libCint.so
#18 0x00cc967d in G__interpret_func ()
   from /opt/pi/ext/dload/root/lib/libCint.so
#19 0x00cb6de8 in G__getfunction () from /opt/pi/ext/dload/root/lib/libCint.so
#20 0x00dba43c in G__getstructmem () from /opt/pi/ext/dload/root/lib/libCint.so
#21 0x00db0228 in G__getvariable () from /opt/pi/ext/dload/root/lib/libCint.so
#22 0x00c8aec1 in G__getitem () from /opt/pi/ext/dload/root/lib/libCint.so
#23 0x00c94a49 in G__getexpr () from /opt/pi/ext/dload/root/lib/libCint.so
#24 0x00d26999 in G__exec_statement ()
   from /opt/pi/ext/dload/root/lib/libCint.so
#25 0x00cca8fc in G__interpret_func ()
   from /opt/pi/ext/dload/root/lib/libCint.so
#26 0x00cb6f58 in G__getfunction () from /opt/pi/ext/dload/root/lib/libCint.so
#27 0x00c8afb1 in G__getitem () from /opt/pi/ext/dload/root/lib/libCint.so
#28 0x00c94a49 in G__getexpr () from /opt/pi/ext/dload/root/lib/libCint.so
#29 0x00c9b85d in G__calc_internal ()
   from /opt/pi/ext/dload/root/lib/libCint.so
#30 0x00d35b83 in G__process_cmd () from /opt/pi/ext/dload/root/lib/libCint.so
#31 0x0063bc63 in TCint::ProcessLine ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#32 0x0063b87f in TCint::ProcessLineSynch ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#33 0x005845b2 in TApplication::ExecuteFile ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#34 0x00584b4c in TApplication::ProcessFile ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#35 0x005817e2 in TApplication::ProcessLine ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#36 0x0011eb87 in TRint::HandleTermInput ()
   from /opt/pi/ext/dload/root/lib/libRint.so
#37 0x0011c6f5 in TTermInputHandler::Notify ()
   from /opt/pi/ext/dload/root/lib/libRint.so
#38 0x0011f7b4 in TTermInputHandler::ReadNotify ()
   from /opt/pi/ext/dload/root/lib/libRint.so
#39 0x00685dab in TUnixSystem::CheckDescriptors ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#40 0x00686935 in TUnixSystem::DispatchOneEvent ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#41 0x005ea194 in TSystem::InnerLoop ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#42 0x005ee091 in TSystem::Run () from /opt/pi/ext/dload/root/lib/libCore.so
#43 0x0057faa8 in TApplication::Run ()
   from /opt/pi/ext/dload/root/lib/libCore.so
#44 0x0011f34c in TRint::Run () from /opt/pi/ext/dload/root/lib/libRint.so
#45 0x08048d93 in main ()
===========================================================

It is possible that it is a bug, so I post it :slight_smile:

None of the developers can confirm this bug?

I haven’t tested it yet, but it seems that in TFITSHDU.cxx the faulty part is

   if (mat) mat->Use(height, width, layer_pixels);

   delete [] layer_pixels;
   return mat;
} 

where delete [] layer_pixels deletes something where mat points after mat->Use(). So removing this delete should help…

This fix helps. Anyone interested in fixing it in the trunk?

Hi,

What is the patch you are proposing? Did you double check it does not result in a memory leak?

Philippe.

No, I haven’t double checked. However, without the mentioned delete, tutorial example does not crash. With the delete it does crash…

Hi,

humm … I must be missing something obvious, I can not find those 3 lines of code anywhere in the tutorials (nor can I find the file TFITSHDU.cxx) … Where did you find them?

Thanks,
Philippe.

graf2d/fitsio/src/TFITS.cxx
TMatrixD* TFITSHDU::ReadAsMatrix(Int_t layer, Option_t *opt)

Yep, sorry, I copied the filename from the html, removing “.html”: root.cern.ch/root/html/src/TFITS … tml#PKC60C but it is in true TFITS.cxx

In principle I agree with LeWhoo.
The “layer_pixels” must NOT be deleted, if it is used afterwards.
The “TMatrixD::Use” method does NOT copy the given “data”, it simply registers the pointer.
So, maybe this fix would do:

[code] // …

if (mat) mat->Use(height, width, layer_pixels); // preserve "layer_pixels"
else delete [] layer_pixels; // no need to keep “layer_pixels”

return mat;
}[/code]

[quote]The “layer_pixels” must NOT be deleted, if it is used afterwards.[/quote]Well actually, if it not deleted there, we would need to keep track somewhere-else a the TMatrix does not take ownership. So, in my opinion, the best solution is to copy the data into the TMatrix rather than sharing the layer_pixels array (this is implemented in the trunk).

Cheers,
Philippe.

You are right, there is no way to “SetOwner”.
Maybe one could add such method?
Or maybe one could add an additional parameter “IsOwner” (defaults to kFALSE, for backwards compatibility reasons) to the “Use” method?