NOAA-EMC / NCEPLIBS-grib_util

This is a collection of NCEP GRIB related utilities.
Other
19 stars 11 forks source link

upgrading to jasper-4.0.0 causes test error #273

Closed edwardhartnett closed 11 months ago

edwardhartnett commented 11 months ago
run_copygb2_tests.sh: line 17: 15043 Aborted                 (core dumped) ../src/copygb2/copygb2 -g "30 6 0 0 0 0 0 0 1473 1025 12190000 226541000 8 25000000 265000000 5079000 5079000 0 64 25000000 25000000" -i"1 1" -x data/ref_gdaswave.t00z.wcoast.0p16.f000.grib2 test_gdaswave_2.ip.grib2
7/7 Test #7: run_copygb2_tests.sh .............***Failed    1.12 sec

*** Running copygb2 test
g2c_compare 1.7.0 comparing data/ref_gdaswave.t00z.wcoast.0p16.f000.grib2 and test_gdaswave_2.grib2.
maximum memory limit (10000000) would be exceeded
cannot encode main body
jpc_encode failed
 jpcpack: ERROR Packing JPC =           -3
 jpcpack: Retrying....
copygb2: /home/runner/work/NCEPLIBS-grib_util/NCEPLIBS-grib_util/jasper/src/libjasper/base/jas_init.c:505: jas_init_library: Assertion `!jas_global.initialized' failed.

Program received signal SIGABRT: Process abort signal.
edwardhartnett commented 11 months ago

OK this was hard to track down, because it was only happening in the runs that built with g2c and used the g2c_compare utility. This utility has nothing to do with the bug, but this test was not even being run elsewhere. Now I have added this test to all runs (#275).

So the problem is here:

+ ../src/copygb2/copygb2 -g '30 6 0 0 0 0 0 0 1473 1025 12190000 226541000 8 25000000 265000000 5079000 5079000 0 64 25000000 25000000' '-i1 1' -x data/ref_gdaswave.t00z.wcoast.0p16.f000.grib2 test_gdaswave_2.ip.grib2
maximum memory limit (10000000) would be exceeded
cannot encode main body
jpc_encode failed
 jpcpack: ERROR Packing JPC =           -3
 jpcpack: Retrying....
copygb2: /home/runner/work/NCEPLIBS-grib_util/NCEPLIBS-grib_util/jasper/src/libjasper/base/jas_init.c:505: jas_init_library: Assertion `!jas_global.initialized' failed.

THis is run by the run_copygb2_tests.sh script, which says:

# Copy GRIB2 file.
../src/copygb2/copygb2 -x data/ref_gdaswave.t00z.wcoast.0p16.f000.grib2 test_gdaswave_2.grib2

# Invoke interpolation logic.
../src/copygb2/copygb2 -g "30 6 0 0 0 0 0 0 1473 1025 12190000 226541000 8 25000000 265000000 5079000 5079000 0 64 25000000 25000000" -i"1 1" -x data/ref_gdaswave.t00z.wcoast.0p16.f000.grib2 test_gdaswave_2.ip.grib2

@AlexanderRichert-NOAA you added this test. Why do you think it is failing so dramatically with an upgrade to jasper?

AlexanderRichert-NOAA commented 11 months ago

In g2, it encodes a memory limit of 10,000,000 bytes for jasper, but only for version 3 and up. When the limit gets tripped, it appears to have the effect of preventing one of the uninitializations of jas_global, hence the assertion error (not sure on the details of why that's the case). Note that the 10,000,000 byte limit is much smaller than the jasper default of 1,073,741,824, and there is a mechanism within jasper to limit the max memory to half the total system memory, plus it spits out warnings for high memory usage, so it's probably safe to set that number much higher in g2, or possibly skip setting it in g2 altogether and use jasper's built-in limits. On my personal computer, the total memory usage and run time for the copygb2 unit test are almost identical between jasper 2.0.33 and jasper 4.0.0, so I don't think that the higher versions of jasper are doing anything "bad" here, I think it's just a function of the limit in g2 for the -DJASPER3 case.

AlexanderRichert-NOAA commented 11 months ago

Note the same setting appears in dec_jpeg2000.c as well, also under #ifdef JASPER3.

edwardhartnett commented 11 months ago

OK, I'll try removing those and we'll see what happens...

edwardhartnett commented 11 months ago

Problem is, after taking it out, jasper spits out all kinds of messages complaining that it was not set:

warning: The application program did not set the memory limit for the JasPer library.
< warning: The JasPer memory limit is being defaulted to a value that may be inappropriate for the system.  If the default is too small, some reasonable encoding/decoding operations will fail.  If the default is too large, security vulnerabilities will result (e.g., decoding a malicious image could exhaust all memory and crash the system.
< warning: setting JasPer memory limit to 3629117440 bytes
AlexanderRichert-NOAA commented 11 months ago

Hm, I guess that would have been too easy... Why don't we try setting the default in g2 to something in the 1-2 billion range since that seems consistent with what jasper offers up as defaults. Not to overcomplicate things, but, what about adding an environment variable, say, $G2C_JASPER_MAX_MEM, that would let us override whatever we set as default in g2? I'm trying to think of some way of avoiding "magic numbers" in our code.