Exiv2

This was reported by private email from Mikayel Egibyan.

If you iterate over the Exif meta-data doing to_string(exifData[i->key()]) many-many times (more than 1000 times) on the same stream/kernel, there is a memory crash.

I think you have a mixed up build environment. Normally, I use static everywhere, or dynamic everywhere.

I believe it is possible to build static DLLs and a static application and use static C run-time. I don't think I've ever done that.

Is there a reason not to use the /MD+DLL build environments in msvc2005?

Mikayel

As you know, I always want to help. I found this blog: http://www.davidlenihan.com/2008/01/choosing_the_correct_cc_runtim.html in which the author says:

• Use the debug version only internally, release for anything that could be used by customers
• Use the DLL version except in special situations
• Use consistent settings throughout your project (the runtime library setting can be done per source file)
• Don't worry if your runtime library settings match other libraries you use (unless the library comes in multiple runtime library versions)

That article references the following article in which Microsoft warn against using C runtime objects across DLL boundaries. Regrettably, that's what you are doing: https://msdn.microsoft.com/en-us/library/ms235460.aspx

It seems to me that using /MT (or /MTd) is only suitable for very small applications in which you want to bind the whole application into a single .exe. When you build with /MT, a copy of the run-time library is bundled into the object you build. When you build with /MD, you are using a process-wide run-time library which is a DLL. That DLL changes for different versions of MSVC. For example, if you are using Visual Studio 2005, it is called MSVCR80.dll

I don't know anything about the application from which you are calling libexiv2. You should build that application with /MD and use the DLLs generated by msvc2005. If you cannot change that application, then I believe you will have to link the static libraries generated by msvc2005. However, I don't think you can say "generate DLLs AND use /MT" because you are violating the conditions described above by Microsoft.

Thanks much Robin for your help! I value it!

The solutions is to easy to discover quickly:

NEVER pass the result of Exiv2 library to 3rd party library, application, dll, etc... without making a copy.
Basically you need to ensure that nothing else but Exiv2 deletes the allocated memory. This is true for all types, especially strings.

Mikayel

Thanks, Mikayel

I haven't understood your fix. Can you modify exifprint.cpp to demonstrate your fix, please?

Pardon me, I didn't notice your message.

Here is the way that I copy the output of Exiv2 before I pass it further:

    size_t len = strlen(str) + 1;
    char *buf = new char[len];
    strcpy(buf, str);
    return but;

I'll update msvc2005/ReadMe.txt with this ingenious tip. So the recipe is "copy the string" (as you said).

Incidentally, I looked up the auto_ptr pattern in wikipedia. https://en.wikipedia.org/wiki/Auto_ptr. It contains the following ominous warning:

may not be used in STL containers that may perform element copies in their operations

The auto_ptr are clearly a tricky customer, and the debugger doesn't understand it either.

The auto_ptr is used extensively by exiv2. I believe auto_ptr is something to ensure that memory doesn't leak when exceptions are thrown. A very good example of a problem being created to solve a problem. Exception handling is a poor error handling strategy. It is an invisible goto that crosses function boundaries. I much prefer returning error codes. Few engineers agree with me, however I am entitled to my opinion. In this case, the exception is being thrown across DLL boundaries and the DLLs are using static libraries. So the Value auto_ptr was allocated by the DLL and deleted by the host process which is using a different heap.

Mikayel

I really don't believe your fix solves this matter. You may have found a way of making your code stable, however I don't believe you have resolved the underlying issue. Please read the following (mostly frightening and blood curling discussion): http://stackoverflow.com/questions/16736195/inline-class-constructor-to-avoid-vc-memory-crash

The article referenced above claims that some relief from this problem arrived with MSVC 2012. If you're using MSVC 2012 or later, perhaps you are being spared the worst cases of the situation. It is my opinion that /MT DLLs are only suitable for small applications and should be avoided. I recommend that you link all your code and DLLs with /MD. I urge you to discuss this with your company's software architects. I am willing to join that discussion on Skype (or phone, Google+ or FaceTime) if you wish.

Looking at this situation from an Exiv2 viewpoint, I feel the only correct way to fix this issue is for the host application to pass pointers to malloc/free/realloc functions to be used by Exiv2. This is a non-trivial change to Exiv2. The default values of these functions would be the system's default implementation. So existing Exiv2 applications would require no changes. Other standard C library functions such as fopen/fclose/fwrite/fread... may benefit from similar treatment.

I have this mostly solved. I say mostly because the solution is incomplete, however I'll explain what else is necessary later.

The solution is for the host application to define the functions to be used by the library to allocate/free memory. So exifprint.cpp becomes:

// Sample program to print the Exif metadata of an image

#include <exiv2/exiv2.hpp>

#include <iostream>
#include <iomanip>
#include <cassert>

int main(int argc, char* const argv[])
try {

    if (argc != 2) {
        std::cout << "Usage: " << argv[0] << " file\n";
        return 1;
    }

    if ( strcmp(argv[1],"--version") == 0 ) {
        exv_grep_keys_t keys;
        Exiv2::dumpLibraryInfo(std::cout,keys);
        return 0;
    }

    Exiv2::Image::AutoPtr image = Exiv2::ImageFactory::open(argv[1]);
    assert(image.get() != 0);
    image->readMetadata();

    Exiv2::ExifData &exifData = image->exifData();
    if (exifData.empty()) {
        std::string error(argv[1]);
        error += ": No Exif data found in the file";
        throw Exiv2::Error(1, error);
    }

    Exiv2::setMemoryFunctions(::malloc,::free); // <----- tell Exiv2 to use our malloc and free
    for (Exiv2::ExifData::const_iterator i = exifData.begin(); i != exifData.end(); ++i) {
        const char* tn = i->typeName();
        std::cout << std::setw(44) << std::setfill(' ') << std::left
                  << i->key() << " " 
                  << "0x" << std::setw(4) << std::setfill('0') << std::right
                  << std::hex << i->tag() << " " 
                  << std::setw(9) << std::setfill(' ') << std::left
                  << (tn ? tn : "Unknown") << " " 
                  << std::dec << std::setw(3)
                  << std::setfill(' ') << std::right
                  << i->count() << "  " 
                  << std::dec << i->value()
                  << "\n";
    }
    Exiv2::setMemoryFunctions(NULL,NULL); // <-- restore Exiv2 default functions

    return 0;
}
//catch (std::exception& e) {
//catch (Exiv2::AnyError& e) {
catch (Exiv2::Error& e) {
    std::cout << "Caught Exiv2 exception '" << e.what() << "'\n";
    return -1;
}

1 Declare Exiv2::setMemoryFunctions()

Modify include/exiv2/exiv2.hpp as follows:

...
#include "xmpsidecar.hpp" 

typedef void* (*malloc_fp)(std::size_t);
typedef void  (*free_fp  )(void*);

namespace Exiv2 {
    static malloc_fp _malloc = NULL;
    static free_fp   _free   = NULL;
    EXIV2API void setMemoryFunctions(malloc_fp malloc, free_fp free);
}

    // replacement of a minimal set of functions:
    void* operator new(std::size_t sz) {
        // std::printf("new called, size = %zu\n",sz);
        return Exiv2::_malloc ? Exiv2::_malloc(sz) : std::malloc(sz);
    }

    void operator delete(void* ptr)
    {
        try {
            // std::puts("delete called");
            if ( Exiv2::_free ) Exiv2::_free(ptr);
            else                std::free(ptr);
        } catch ( std::exception&  ) {}
    }
#endif                                  // #ifndef EXIV2_HPP_

2 Define setMemoryFunctions in basicio.cpp

Update src/basicio.cpp as follows:

# include <windows.h>
# include <io.h>
#endif

#include <exiv2/exiv2.hpp>

// *****************************************************************************
// class member definitions
namespace Exiv2 {

#ifdef EXV_BUILDING_LIB
    EXIV2API void setMemoryFunctions(malloc_fp malloc, free_fp free)
    {
        _malloc=malloc;
        _free  =free  ;
    }
#endif
    BasicIo::~BasicIo()
    {
    }

Good, isn't it?

It is possible to update MSVC2005 to compile this code when building /MT DLLs. I will not support this for the following reasons:

1) There are more changes required:

Exiv2 allocates memory with operator new, ::malloc(), ::calloc(), std::malloc(), ::realloc(). I will have to identify and modify all that code.

2) All the sample applications will require modification

3) If I move the calls to Exiv2::setMemoryFunctions() to the first and last lines of exifprint.cpp, the application asserts during exit. This is some synchronisation issue with the conflicting static libraries which I don't want to debug.

4) I am sure there will be more crashes to be investigated before the test suite performs perfectly.

5) I do not wish to encourage anybody to use /MT DLLs.

6) There are users with their own MSVC build environments. C++ code changes to deal with /MT DLLs will probably damage their build environments.