Windows DllMain initialization broken (6.9.0-3)

[Myrsloik]

If you use any of the ImageMagick dlls directly on windows (in my case through Magick++ but MagickWand will show the same issue) MagickCoreGenesis() will be called automatically in DllMain(). This is an issue for several reasons:

1. Since MagickCoreGenesis() is already called before any other code can run and subsequent calls are ignored there's no way to set the execution path.
2. The automatically supplied path in MagickCoreGenesis() is always INVALID. Combined with 1 this is a huge frustration if all you want to do is to distribute the ImageMagick dlls with an appliction.

The offending code is in nt-base.c, the marked part needs to be deleted. MagickCoreGenesis() actually needs a path with a filename, stripping the filename ensures the path will always be ignored.

In nt-base.c:

Code: Select all

BOOL WINAPI DllMain(HINSTANCE handle,DWORD reason,LPVOID lpvReserved)
{
...
      count=(ssize_t) GetModuleFileNameW(handle,wide_path,MaxTextExtent);
      if (count != 0)
        {
          char
            *path;

          module_path=create_utf8_string(wide_path);
DELETE FROM HERE
   for ( ; count > 0; count--)
            if (module_path[count] == '\\')
              {
                module_path[count+1]='\0';
                break;
              }
DELETE TO HERE
          MagickCoreGenesis(module_path,MagickFalse);
          ...

There is a second issue that also blocks the automatically supplied path from being valid. Absolute windows paths generally DO NOT start with a backslash. I assume someone tried to be clever and forgot that linux isn't the only OS on earth. Put the marked check in an ifdef so it's not checked on windows.

In magick.c:

Code: Select all

MagickExport void MagickCoreGenesis(const char *path,
  const MagickBooleanType establish_signal_handlers)
{
...
#if defined(MAGICKCORE_WINDOWS_SUPPORT)
  NTWindowsGenesis();
#endif
  /*
    Set client name and execution path.
  */
  (void) GetExecutionPath(execution_path,MaxTextExtent);
  if ((path != (const char *) NULL) &&
  ONLY USE IF NOT WINDOWS
  (*path == *DirectorySeparator) &&
  END ONLY USE IF NOT WINDOWS
      (IsPathAccessible(path) != MagickFalse))
    (void) CopyMagickString(execution_path,path,MaxTextExtent);
  GetPathComponent(execution_path,TailPath,filename);
  (void) SetClientName(filename);
  GetPathComponent(execution_path,HeadPath,execution_path);
  (void) SetClientPath(execution_path);
...

These changes make ImageMagick usable in dll form on windows again. The DllMain() initialization on windows should probably be documented somewhere too.

[dlemstra]

You are correct on both changes, we will apply both changes before the release of ImageMagick 6.9.0-4. You might want to take a look at the following define:

Code: Select all

// Optional: Specify name of the library that contains the xml resource files
// #define MAGICKCORE_LIBRARY_NAME "MyImageMagick.dll"

If you set this define and include the xml resources as embedded resources in your dll you probably don't need to set the path with MagickCoreGenesis. In Magick.NET I undefine 'DllMain' and just call MagickCoreGenesis(NULL,false) from my own DllMain.

[dlemstra]

After checking the code again I think it might be wiser to also #undef the ProvideDllMain. The client name will be set to the name of the dll which will result in these kind of weird error message for our executables:

Code: Select all

C:\>compare -metric rmse logo: logo:
CORE_DB_MagickCore_.dll: missing an image filename `logo:' @ error/compare.c/CompareImagesCommand/937.

The user should call MagickCoreGenesis but can also decide to use the DllMain feature by defining ProvideDllMain.

[Myrsloik]

I agree. Not having the dllmain initialization enabled in the official binaries would also be an improvement.

[Myrsloik]

After a bit of pondering I remembered that you can easily get the name of the executable instead. You may want to change the line

Code: Select all

count=(ssize_t) GetModuleFileNameW(handle,wide_path,MaxTextExtent);

to

Code: Select all

count=(ssize_t) GetModuleFileNameW(NULL,wide_path,MaxTextExtent);

in the DllMain() call. This will make it retrieve the current executable name instead of the current core dll name. It'll at least improve the issue you mentioned and is probably a more sane default.

Reference:
http://msdn.microsoft.com/en-us/library ... 85%29.aspx

[dlemstra]

We already do that in NTGetExecutionPath. This method will be called when MagickCoreGenesis is called with NULL as the value for *path. I will change the call in DllMain to MagickCoreGenesis((const char*) NULL,MagickFalse). I am not sure why setting the 'PATH' is necessary in DllMain but I will not remove it for now.

The 'ProvideDllMain' define will still be changed to undefined in the next release because I think it is better if the executables call MagickCoreGenesis themselves.

[Myrsloik]

You forgot to recompile CORE_RL_magick_.dll (and possibly other files too) for the 6.9.0-4 windows binary release. The provided binaries are identical to 6.9.0-3 so the issue remains there.

I can confirm that it works when I compile my own version from source.