JP2KAK race condition

[adamarc]

A race condition exists in the JP2KAK driver (at least in GDAL version 1.9.0 and with Kakadu version 7.1). The race condition can cause memory corruption in a multi-threaded environment.

The problem is that kdu_cpl_error_message inherits from kdu_message which is not thread safe. It should inherit from kdu_thread_safe_message. See also the Kakadu documentation for kdu_message.

The fix is trivial (and I have verified this with my own locally patched version):

• make kdu_cpl_error_message inherit from kdu_thread_safe_message
• call kdu_thread_safe_message::flush() from within kdu_cpl_error_message::flush()

I am marking this major due to the amount of pain and suffering this problem has caused me.

[warmerdam]

I'll take care of incorporating this.

[warmerdam]

I have applied a change (r25137) in trunk that I think captures what you are suggesting. It is hard for me to know if this is correct since it is hard to trigger the affected code. Could you let me know if this is appropriate. Once I am confident I'll back port it to 1.9 branch as well.

[adamarc]

The call to kdu_thread_safe_message::flush() must occur at the end of kdu_cpl_error_message::flush() - not at the start. This is to ensure that the underlying mutex is only released when memory has been deallocated.

[adamarc]

Additionally. A number of methods need to be exported (by prefixing with KDU_EXPORT) from the Kakadu library in order for the above GDAL changes to be compiled under windows. They are:

kdu_thread_entity::process_jobs() kdu_thread_entity::~kdu_thread_entity() kdu_block::kdu_block() kdu_block::~kdu_block()

[adamarc]

I've passed this request on to THE Kakadu people.

[adamarc]

Having spoken with David at Kakadu he haspointed out that the Windows build problem is probably to do with the unusual way the GDAL Windows build scripts are set up in that they link directly against intermediate .obj files. From David's email:

Some of this seems to be related to the way GDAL has been built against Kakadu. I have not had others report any of these issues. The preferred mechanism for building against Kakadu is to compile "coresys" to get kdu_v71R.dll and kdu_v71D.dll, then to compile "managed" to get kdu_a71R.dll and kdu_a71D.dll. It is also possible to compile static versions of these libraries, which is offered in the Linux makefiles and the OSX build environments but not provided out of the box for MSVC. In any case, applications should be linked against the kdu_v71 and kdu_a71 libraries/dlls and use the headers found in "managed/all_includes". At least, this is the preferred way to build applications against Kakadu. It also means that you should not need to build "apps" at all in order to build your own application against Kakadu -- "apps" is really intended for Kakadu's demo applications. It seems that GDAL relies upon importing all the source/object files directly from their build locations, which seems a rather odd way to do things, and highly dependent on the way Kakadu itself is built.

[Even Rouault]

trunk r27256, branches/1.11 r27257 "JPIPKAK: avoid symbol collision with kdu_cpl_error_message from JP2KAK driver (#4865)"

[Jukka Rahkonen]

Even, do r27256 and r27257 mean that this ticket can be closed now?

[Even Rouault]

Probably