Race Condition Loading TIFF Images

kspaff
2013-12-02
2013-12-05
  • kspaff
    kspaff
    2013-12-02

    Hello, I'm using FreeImage to load TIFF images in a multi-threaded windows application. The application starts loading images immediately in multiple threads (2-5). Occasionally (maybe 1 out of every 5 runs), there is a heap corruption or access violation error coming from FreeImage.

    I'm on Windows using the latest release and VS 2010. Here is the stack frame for the error and the area of the code I think may be responsible:

    Stack Frame

    Code

    I don't know the internals very well, but is it possible that there is a race condition in XTIFFInitialize() (XTIFF.cpp:93) where first_time is being cleared, then another thread is proceeding without setting _ParentExtender?

    Here is the function that loads the images in particular the GetMirroredImageFromDisk() method at line 75

     MFPtr GetMirroredImageFromDisk(const string& fileName) {
            // Bitmap pointer for the image
            FIBITMAP *dib = NULL;
            dib = GenericLoader(fileName.c_str(), 0);
    
            if (dib != NULL && FreeImage_GetBPP(dib) == 16) {
                unsigned int fi_w = FreeImage_GetWidth(dib);
                unsigned int fi_h = FreeImage_GetHeight(dib);
                unsigned int nSrcPitch = FreeImage_GetPitch(dib);
                const BYTE* pSrcLine = FreeImage_GetBits(dib) + nSrcPitch * (fi_h - 1);
                unsigned int indx = 0;
    
                float* pDst = NULL;
                CUDA(cudaHostAlloc(&pDst, fi_w * fi_h * sizeof(float), cudaHostAllocPortable));
    
                for (unsigned int y = 0; y < fi_h; ++y) {
                    for (unsigned int x = 0; x < fi_w; ++x) {
                        const BYTE *src = pSrcLine - y * nSrcPitch;
                        unsigned int low  = 0 | (*(src + x*2)     & 0xFFFF);
                        unsigned int high = 0 | (*(src + x*2 + 1) & 0xFFFF);
                        pDst[indx] =  (float)(high << 8 | low);
                        ++indx;
                    }
                }
                FreeImage_Unload(dib);
                return MFPtr(new MirroredArray<float>(pDst, fi_w, fi_h));
            } else if (dib != NULL && FreeImage_GetBPP(dib) == 32) {
                FREE_IMAGE_TYPE image_type = FreeImage_GetImageType(dib);
                if (image_type != FIT_FLOAT) {
                    cout << "Error, unsupported image, 32bpp float supported" <<
                        endl;
                    exit(-1);
                }
    
                unsigned int fi_w = FreeImage_GetWidth(dib);
                unsigned int fi_h = FreeImage_GetHeight(dib);
                unsigned int nSrcPitch = FreeImage_GetPitch(dib);
    
                float* pDst = NULL;
                CUDA(cudaHostAlloc(&pDst, fi_w * fi_h * sizeof(float), cudaHostAllocPortable));
    
                int dest_row = fi_h - 1;
    
                for (unsigned int y = 0; y < FreeImage_GetHeight(dib); y++) {
                    float *bits = (float *)FreeImage_GetScanLine(dib, y);
                    for (unsigned int x = 0; x < FreeImage_GetWidth(dib); x++) {
                        pDst[dest_row * fi_w + x] = bits[x];
                    }
                    dest_row--;
                }
                FreeImage_Unload(dib);
                return MFPtr(new MirroredArray<float>(pDst, fi_w, fi_h));
            } else if (dib != NULL) {
                FreeImage_Unload(dib);
            }
            return NULL;
        }
    
        /** Generic image loader function, loads an image from file using calls to
        the FreeImage library.
        @param lpszPathName Pointer to the full file name
        @param flag Optional load flag constant
        @return Returns the loaded dib if successful, returns NULL otherwise
        */
        FIBITMAP* GenericLoader(const char* lpszPathName, int flag) {
            FREE_IMAGE_FORMAT fif = FIF_UNKNOWN;
    
            // check the file signature and deduce its format
            // (the second argument is currently not used by FreeImage)
            fif = FreeImage_GetFileType(lpszPathName, 0);
            if(fif == FIF_UNKNOWN) {
                // no signature ?
                // try to guess the file format from the file extension
                fif = FreeImage_GetFIFFromFilename(lpszPathName);
            }
            // check that the plugin has reading capabilities ...
            if((fif != FIF_UNKNOWN) && FreeImage_FIFSupportsReading(fif)) {
                // ok, let's load the file
                FIBITMAP *dib = FreeImage_Load(fif, lpszPathName, flag);
                // unless a bad file format, we are done !
                return dib;
            }
            return NULL;
        }
    

    I am trying to load the TIFFs into a float array using a custom allocator (cudaHostAlloc) and I do some special masking to mirror the behavior of some legacy code.

    Any thoughts on what might be causing this error?

     
  • kspaff
    kspaff
    2013-12-02

    I can also add that the problem does not occur if I run with only one thread or use multiple threads with a mutex such that only one thread can call GetMirroredImageFromDisk() at a time.

     
  • Hervé Drolon
    Hervé Drolon
    2013-12-03

    Hi,

    This is probably a race condition. A similar problem was already encountered with the TagLib singleton,
    see https://sourceforge.net/p/freeimage/discussion/36110/thread/5470b918/

    A quick solution would be to add a call to XTIFFInitialize() inside the FreeImage_Initialise function.
    Can you try this change inside Plugin.cpp :

    // =====================================================================
    // Plugin System Initialization
    // =====================================================================

    //! Extended TIFF Directory GEO Tag Support (see XTIFF.cpp)
    extern void XTIFFInitialize();

    void DLL_CALLCONV
    FreeImage_Initialise(BOOL load_local_plugins_only) {
    if (s_plugin_reference_count++ == 0) {

        // initialise the TagLib singleton
        TagLib& s = TagLib::instance();
    
        // CHANGE IS HERE : sets up the callback procedure for the TIFF module
        XTIFFInitialize();
    
        // internal plugin initialization
    

    If its OK for you, I will make the change to the CVS.

    Hervé

     
    • kspaff
      kspaff
      2013-12-03

      Hi Hervé, this seems to have fixed the problem. Thank you very much.

       
  • Shouldn't this go into InitTIFF (PluginTIFF.cpp) since FreeImage is plugin based and all?

     
  • Hervé Drolon
    Hervé Drolon
    2013-12-05

    Hi Floris,

    Indeed you are right !
    I've fixed the PluginTIFF.cpp code in the CVS.

    Hervé