fabio icon indicating copy to clipboard operation
fabio copied to clipboard

[debci] for 0.7.0 version

Open picca opened this issue 7 years ago • 26 comments

Hello, it seems that there is some issue with the python3 implementation.

you can find the log here for all python[2-3][-dbg] version of fabio.

https://ci.debian.net/data/autopkgtest/unstable/amd64/p/python-fabio/751490/log.gz

cheers

picca avatar Aug 02 '18 06:08 picca

We usually don't test the -dbg packages ... I wonder if it makes sense to still produce them since we have now a package with symbols which is enough for debugging with GDB ?

kif avatar Oct 11 '18 13:10 kif

Hello, I have this error message in the ci (still with the python dbg version):

https://ci.debian.net/packages/p/python-fabio/testing/amd64/

https://ci.debian.net/data/autopkgtest/testing/amd64/p/python-fabio/10371373/log.gz

python3.9-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.

First it seems that it is only triggered with the 1.19.5 version of numpy ???

If I look after this error message on internet, I have a bunch or relevant answer like this one.

https://github.com/inducer/pycuda/issues/233

https://gitlab.tiker.net/inducer/pycuda/-/merge_requests/22/commits?commit_id=16c81fce32adb69fbb9ec70dbba75e040c0c211a

https://github.com/inducer/bpl-subset/compare/0d99742a3613fef0504705897ce6c74193e501e3...3702fb119804dddde5eaf4a254822b891a947104#diff-f649bd645442d73d4a799580395b3c8a74f00780e8e1c37b9f0b66bdf21c2be5

I am wondering if this as some meaning for you...

the culprite could be in numpy 1.19.5, or a fabio bug triggered only with numpy 1.19.5 ...

picca avatar Feb 09 '21 20:02 picca

after investigation the problem comes from these tests testSuite.addTest(codecs.suite())

picca avatar Feb 09 '21 20:02 picca

in the codec this one is the wrong one... testSuite.addTest(test_tifimage.suite())

picca avatar Feb 09 '21 20:02 picca

inside whcih we find this wrong test testsuite.addTest(loadTests(TestTif_LibTiffPic))

picca avatar Feb 09 '21 21:02 picca

the fax2d.tif image seems to cause troubles ???

WARNING:fabio.tifimage:Unable to read /tmp/fabio_testdata_picca/libtiffpic.tar.gz__content/fax2d.tif with TiffIO due to Compressed TIFF images not supported except packbits, trying PIL
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.

picca avatar Feb 09 '21 21:02 picca

If I comment the code whcih use PIL

    def _read_with_pil(self, infile):
        # pilimage = PIL.Image.open(infile)                                                                                                                                              
        # header = self._read_header_from_pil(pilimage)                                                                                                                                  
        # data = pilutils.get_numpy_array(pilimage)                                                                                                                                      
        # frame = self._create_frame(data, header)                                                                                                                                       
        # self.header = frame.header                                                                                                                                                     
        # self.data = frame.data                                                                                                                                                         
        # self._shape = None                                                                                                                                                             
        # self.lib = "PIL"                                                                                                                                                               
	pass

the test pass. Indeed nothing is done.

picca avatar Feb 09 '21 21:02 picca

this line seems to cause the trouble

        data = pilutils.get_numpy_array(pilimage)

picca avatar Feb 09 '21 21:02 picca

By instrumenting the code, it seems that this image is the only one with a bool dtype

WARNING:fabio.tifimage:Unable to read /tmp/fabio_testdata_picca/libtiffpic.tar.gz__content/fax2d.tif with TiffIO due to Compressed TIFF images not supported except packbits, trying PIL
toto :  <class 'bool'>
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.

picca avatar Feb 09 '21 21:02 picca

a modification around the bool type was done in numpy

https://github.com/numpy/numpy/pull/17918

picca avatar Feb 09 '21 21:02 picca

ok, I can reproduce the bug with this small script

import numpy
import PIL.Image
img = PIL.Image.open("fax2d.tif")
numpy.asarray(img, bool)

python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed. Abandon

picca avatar Feb 09 '21 21:02 picca

In order to try to understand something I ran it wit the normal Python

$ python3
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> numpy.asarray(img, bool)
array(True)
>>> img
<PIL.TiffImagePlugin.TiffImageFile image mode=1 size=1728x1082 at 0x7F0495798A30>
>>> numpy.asarray(img, numpy.bool)
array([[False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False],
       ...,
       [False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False],
       [False, False, False, ..., False, False, False]])

the result is different, if I use numpy.bool instead of bool. to me numpy.bool seems better with this version of numpy 1.19.5 If I remember correctly something changed around numpy.bool with numpy 1.20 :(. so be careful with this...

picca avatar Feb 09 '21 21:02 picca

With the python3-dbg version, both case create troubles.

picca@2a02-8420-6c55-6500-d012-4688-0bee-a0c6:/tmp/fabio_testdata_picca/libtiffpic.tar.gz__content$ python3-dbg
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> numpy.asarray(img, numpy.bool)
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.
Abandon
picca@2a02-8420-6c55-6500-d012-4688-0bee-a0c6:/tmp/fabio_testdata_picca/libtiffpic.tar.gz__content$ python3-dbg
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> import numpy
>>> numpy.asarray(img, bool)
python3-dbg: ../Objects/typeobject.c:3241: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed.
Abandon

picca avatar Feb 09 '21 22:02 picca

Still playing with all this... I will open the file with PIL, convert it to "F" and see if this is better.

$ python3-dbg
Python 3.9.1+ (default, Feb  5 2021, 13:46:56) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL.Image
>>> img = PIL.Image.open("fax2d.tif")
>>> img
<PIL.TiffImagePlugin.TiffImageFile image mode=1 size=1728x1082 at 0x7F292454F280>
>>> img2 = PIL.Image.open("fax2d.tif")
>>> img.convert("F")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/PIL/Image.py", line 904, in convert
    self.load()
  File "/usr/lib/python3/dist-packages/PIL/TiffImagePlugin.py", line 1088, in load
    return self._load_libtiff()
  File "/usr/lib/python3/dist-packages/PIL/TiffImagePlugin.py", line 1192, in _load_libtiff
    raise OSError(err)
OSError: -9
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A116E0>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A11410>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A110F0>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A11320>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A11690>
>>> img.convert("F")
<PIL.Image.Image image mode=F size=1728x1082 at 0x7F2923A116E0>
>>> 

amazing, the first attempt to convert the image produce an OSError (-9), then the next attempt works. I think that I am on something...

picca avatar Feb 09 '21 22:02 picca

this error -9 come from the decoder C part of Pillow the meaning is

/* Errcodes */
#define IMAGING_CODEC_END        1
#define IMAGING_CODEC_OVERRUN   -1
#define IMAGING_CODEC_BROKEN    -2
#define IMAGING_CODEC_UNKNOWN   -3
#define IMAGING_CODEC_CONFIG    -8
#define IMAGING_CODEC_MEMORY    -9

picca avatar Feb 09 '21 22:02 picca

we can find this error message here

$ rgrep IMAGING_CODEC_MEMORY
libImaging/XbmEncode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:                state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipEncode.c:                        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/GifEncode.c:            state->errcode = IMAGING_CODEC_MEMORY;\
libImaging/GifEncode.c:                        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/SgiRleDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/SgiRleDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/SgiRleDecode.c:        err = IMAGING_CODEC_MEMORY;
libImaging/Jpeg2KDecode.c:                state->errcode = IMAGING_CODEC_MEMORY;
libImaging/Imaging.h:#define IMAGING_CODEC_MEMORY    -9
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:        state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/TiffDecode.c:                state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipDecode.c:            state->errcode = IMAGING_CODEC_MEMORY;
libImaging/ZipDecode.c:                state->errcode = IMAGING_CODEC_MEMORY;

picca avatar Feb 09 '21 22:02 picca

Running the code in gdb, I found that the MEMORY error comes from the _decodeStrip method

int _decodeStrip(Imaging im, ImagingCodecState state, TIFF *tiff) {
    INT32 strip_row;
    UINT8 *new_data;
    UINT32 rows_per_strip, row_byte_size;
    int ret;

    ret = TIFFGetField(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip);
    if (ret != 1) {
        rows_per_strip = state->ysize;
    }
    TRACE(("RowsPerStrip: %u \n", rows_per_strip));

    // We could use TIFFStripSize, but for YCbCr data it returns subsampled data size
    row_byte_size = (state->xsize * state->bits + 7) / 8;

    /* overflow check for realloc */
    if (INT_MAX / row_byte_size < rows_per_strip) {
        state->errcode = IMAGING_CODEC_MEMORY;                         <-------------   this one
        return -1;
    }

picca avatar Feb 09 '21 22:02 picca

After the call of TIFFGetField(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip);

(gdb) p rows_per_strip 
$7 = 4294967295

the value of rows_per_strip seems strange

(gdb) p row_byte_size 
$1 = 216
(gdb) p state->xsize
$2 = 1728
(gdb) p state->bits
$3 = 1

picca avatar Feb 09 '21 22:02 picca

during the compilation of Pillow, I can see these warning for the TiffDecode.c

src/libImaging/TiffDecode.c: In function ‘_decodeStripYCbCr’:
src/libImaging/TiffDecode.c:213:22: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32’ {aka ‘unsigned int’} [-Wsign-compare]
  213 |     if (state->xsize != img.width || state->ysize != img.height) {
      |                      ^~
src/libImaging/TiffDecode.c:213:51: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32’ {aka ‘unsigned int’} [-Wsign-compare]
  213 |     if (state->xsize != img.width || state->ysize != img.height) {
      |                                                   ^~
src/libImaging/TiffDecode.c: In function ‘ImagingLibTiffDecode’:
src/libImaging/TiffDecode.c:498:41: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
  498 |                 for (tile_y = 0; tile_y < current_tile_length; tile_y++) {
      |                                         ^

picca avatar Feb 09 '21 22:02 picca

to my opinion getting rid of PIL would be nice and replace it with wand.

I could read the image and convert it with no error

import wand.image

img = wand.image.Image(filename="fax2d.tif")
arr = numpy.array(img)
>>> arr.shape
(1082, 1728, 1)

See you :))

picca avatar Feb 10 '21 00:02 picca

This issue should be reported to the pillow project then, no?

We probably dont care about bool image at synchrotron so this image could be skipped from our tests (while it is not fixed in pillow).

vallsv avatar Feb 10 '21 08:02 vallsv

for mask bool images are great :)).

Maybe removing all the specific code for tiff images, should help reduce the maintenance of fabio :)). I agree that this bug report should be reported to pillow upstream. It was late, and I was fed-up. I was in a hurry in order to salvage fabio from exclusion of the next Debian stable release.

cheers

Fred

picca avatar Feb 10 '21 10:02 picca

Thanks a lot Fred for this investigation. I agree with the bug should be reported to Pillow Bool images are often used to store masks ...

kif avatar Feb 10 '21 10:02 kif

My initial comment remains: do we need to maintain the dbg version if nobody uses them ?

kif avatar Feb 10 '21 10:02 kif

Yes, I think, because this trigger real bugs :)), it is nice to have this run in order to show leak of ressources or other probles :)

I have a doubt about this bug, because it seems that it was not present with numpy 1.19.4, but has I said previously, the output of the convert method is strange depending on the dtype.

The C code of Pillow, its not simple especillay if you consider CVE. at least imagemagick has a good decurity support :).

picca avatar Feb 10 '21 10:02 picca

We could convert binary files into uint8 dtype, that would be easy on out side.

kif avatar Feb 10 '21 13:02 kif