Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using GR plugins without setting PATH on Windows #489

Closed
mkitti opened this issue Nov 4, 2022 · 15 comments
Closed

Using GR plugins without setting PATH on Windows #489

mkitti opened this issue Nov 4, 2022 · 15 comments

Comments

@mkitti
Copy link
Contributor

mkitti commented Nov 4, 2022

Currently, we set ENV["PATH"] on Windows since we need to communicate the DLL search path to GR so that it can dynamically load plugins.

ENV["PATH"] = join((lp, get(ENV, "PATH", "")), ';')

The normal Windows search order is specified in the documentation with PATH being sixth in the priority list.

https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order#search-order-for-desktop-applications

If SafeDllSearchMode is disabled, the search order is as follows:

The directory from which the application loaded.

  1. The current directory.
  2. The system directory. Use the GetSystemDirectory function to get the path of this directory.
  3. The 16-bit system directory. There is no function that obtains the path of this directory, but it is searched.
  4. The Windows directory. Use the GetWindowsDirectory function to get the path of this directory.
  5. The directories that are listed in the PATH environment variable. Note that this does not include the per-application path specified 6. by the App Paths registry key. The App Paths key is not used when computing the DLL search path.

As @giordano pointed out, this "looks like a bad idea".

The GR plugin loading code is located here:
https://github.com/sciapp/gr/blob/9aa4f55f75101b76ba8e4406b78601e410642cc6/lib/gks/plugin.c#L46-L55

#ifdef _WIN32
  handle = LoadLibrary(pathname);
  if (handle == NULL)
    {
      grdir = gks_getenv("GRDIR");
      if (grdir == NULL) grdir = GRDIR;
      snprintf(grbin, MAXPATHLEN, "%s/bin", grdir);
      SetDllDirectory(grbin);
      handle = LoadLibrary(pathname);
    }

Noticeably, this uses SetDllDirectory which maps to either SetDllDirectoryA or SetDllDirectoryW based on the value of the environment variable GRDIR.

Setting GRDIR works for a single directory. The following works in VSCode after commenting out line 38 of funcptrs.jl above.

julia> using GR

julia> x = 0:2π/360:2π; y = sin.(x);

julia> ENV["GRDIR"] = dirname(dirname(GR.GRPreferences.GR_jll.libGR_path))
"~\\.julia\\artifacts\\e40afe22fcfc9fe1caea4af17a4016e7ef36cd66"

julia> plot(x,y) # No Error

julia> contains(ENV["PATH"], ENV["GRDIR"])
false

If we did not set GRDIR, this error occurs: GKS: svgplugin.dll: can't load library, error 126 (0x7e).

While setting GRDIR for a single directory works, there may be many directories to include in the DLL search path:

julia> length(GR.GRPreferences.GR_jll.LIBPATH_list)
35

One strategy then would be for us to call SetDllDirectoryA directly. This works in VSCode:

julia> using GR

julia> kernel32 = :kernel32
:kernel32

julia> @ccall kernel32.SetDllDirectoryA(dirname(GR.GRPreferences.GR_jll.libGR_path)::Cstring)::UInt8
0x01

julia> x = 0:2π/360:2π; y = sin.(x);

julia> plot(x,y) # No error

julia> haskey(ENV, "GRDIR")
false

julia> contains(ENV["PATH"], dirname(dirname(GR.GRPreferences.GR_jll.libGR_path)))
false

We could of course do this for all directories in LIBPATH_list. This also works in VSCode.

julia> using GR

julia> kernel32 = :kernel32
:kernel32

julia> all([@ccall kernel32.SetDllDirectoryA(d::Cstring)::Bool for d in GR.GRPreferences.GR_jll.LIBPATH_list])
true

julia> x = 0:2π/360:2π; y = sin.(x);

julia> plot(x,y)

cc:@t-bltg

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

We might want to use a UTF-16 version.

julia> using GR

julia> kernel32 = :kernel32
:kernel32

julia> all([@ccall kernel32.SetDllDirectoryW(push!(transcode(UInt16, d),0x0000)::Ptr{UInt16})::Bool for d in GR.GRPreferences.GR_jll.LIBPATH_list])
true

julia> x = 0:2π/360:2π; y = sin.(x);

julia> plot(x,y) # No errors

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

Actually SetDllDirectoryW may only keep the last directory added. Perhaps we need AddDllDirectory

https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-adddlldirectory

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

julia> using GR

julia> kernel32 = :kernel32

julia> [@ccall kernel32.AddDllDirectory(push!(transcode(UInt16, d),0x0000)::Ptr{UInt16})::Ptr{Nothing} for d in reverse(GR.GRPreferences.GR_jll.LIBPATH_list)]
35-element Vector{Ptr{Nothing}}:
 Ptr{Nothing} @0x00000001cf47b0d0
 Ptr{Nothing} @0x00000001cf47b4e0
 Ptr{Nothing} @0x00000001cf47b680
 Ptr{Nothing} @0x00000001cf47cd40
 Ptr{Nothing} @0x00000001cf47a4a0
 Ptr{Nothing} @0x00000001cf47cad0
 Ptr{Nothing} @0x00000001cf47a3d0
 Ptr{Nothing} @0x00000001cf47c930
 Ptr{Nothing} @0x00000001cf47a300
 Ptr{Nothing} @0x00000001cf47cba0
 
 Ptr{Nothing} @0x00000001cf47a7e0
 Ptr{Nothing} @0x00000001cf47b270
 Ptr{Nothing} @0x00000001cf47c790
 Ptr{Nothing} @0x00000001cf47ce10
 Ptr{Nothing} @0x00000001cf47a8b0
 Ptr{Nothing} @0x00000001cf47b340
 Ptr{Nothing} @0x00000001cf474710
 Ptr{Nothing} @0x00000001cf47b8f0
 Ptr{Nothing} @0x00000001cf47cfb0
 Ptr{Nothing} @0x00000001cf47b9c0

julia> @ccall kernel32.SetDefaultDllDirectories(0x00000400::UInt32)::Bool
true

julia> x = 0:2π/360:2π; y = sin.(x);

julia> plot(x,y)

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

@jheinen
Copy link
Owner

jheinen commented Nov 4, 2022

The GR plugin loading code is located here: https://github.com/sciapp/gr/blob/9aa4f55f75101b76ba8e4406b78601e410642cc6/lib/gks/plugin.c#L46-L55

@mkitti : This plugin is only used by us, e.g. when we develop a new GKS output driver. In normal GR operation, this method is never used. The plugins are called directly here.

So, by using a workstation type 301 (or "plugin"), programmers can then develop and test a new output driver (built as a shared object) without having to completely rebuild the GKS each time.

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

301 leads to gks_drv_plugin.

gks_drv_plugin seems to be defined at
https://github.com/sciapp/gr/blob/9aa4f55f75101b76ba8e4406b78601e410642cc6/lib/gks/plugin.c#L239

That calls load_library: https://github.com/sciapp/gr/blob/9aa4f55f75101b76ba8e4406b78601e410642cc6/lib/gks/plugin.c#L32

Which leads to the code I pointed to earlier, right?

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

Similarly, 382 leads to gks_svg_plugin being called here:

https://github.com/sciapp/gr/blob/9aa4f55f75101b76ba8e4406b78601e410642cc6/lib/gks/gks.c#L272

gks_svg_plugin is then implemented here:

https://github.com/sciapp/gr/blob/9aa4f55f75101b76ba8e4406b78601e410642cc6/lib/gks/plugin.c#L239

That in turn dynamically loads svgplugin.dll, or do I misunderstand the code?

@jheinen
Copy link
Owner

jheinen commented Nov 4, 2022

That in turn dynamically loads svgplugin.dll, or do I misunderstand the code?

That's right, yes. Which means, that GRDIR must point to the "real" GR directory.

@t-bltg
Copy link
Contributor

t-bltg commented Nov 4, 2022

Thanks for working on a clean fix @mkitti, and the corresponding analysis: this is the right way to proceed.

I had seen https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order#search-order-for-desktop-applications before, but without a windows machine, and having never user windows, it was difficult to work on this issue.

Actually SetDllDirectoryW may only keep the last directory added. Perhaps we need AddDllDirectory

To me unless the gr process is isolated (and communicates through a socket - afaik that's what we do not do in julia), we need to use AddDllDirectory since forcing the search path with SetDllDirectoryW can affect the loading of deferred dlopened libs in julia ? Or you maybe need to clean the search path immediately after usage, limiting the side effects ...

@jheinen
Copy link
Owner

jheinen commented Nov 4, 2022

I try to implement the AddDllDirectory / RemoveDllDirectory logic asap. But I can't test it before Monday. As mentioned before, GRDIR must point to the GR plugin directory. We should leave this functionality in GR to not break things for other languages (C/C++, Python, R, ...).

@jheinen
Copy link
Owner

jheinen commented Nov 4, 2022

It seems to me, that we don't need any *DllDirectory function, as long as GRDIR is set. According to the documentation the name of the DLL module in LoadLibraryA can be a full path:

If the string specifies a full path, the function searches only that path for the module.

In any case, I will use the LoadLibraryW function to allow path names containing 16-bit Unicode characters.
I will check this on Monday.

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

What is GRDIR supposed to be set to when using GR_jll?

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

Also, my suggestion is to use AddDllDirectory here in GR.jl not upstream in gr framework. We may need to use AddDllDirectory here since there are multiple library paths.

@mkitti
Copy link
Contributor Author

mkitti commented Nov 4, 2022

To be specific, my suggestion is that we replace ENV["PATH"] = join((lp, get(ENV, "PATH", "")), ';')

ENV["PATH"] = join((lp, get(ENV, "PATH", "")), ';')

with

kernel32 = :kernel32
@ccall kernel32.SetDefaultDllDirectories(0x00000400::UInt32)::Bool
[@ccall kernel32.AddDllDirectory(push!(transcode(UInt16, d),0x0000)::Ptr{UInt16})::Ptr{Nothing} for d in GRPreferences.GR_jll.LIBPATH_list]

I think it might make sense to also do

ENV["GRDIR"] = dirname(dirname(GRPreferences.GR_jll.libGR))

Currently, when using GR_jll, the GRDIR environment variable is not set all.

julia> haskey(ENV, "GRDIR")
false

@mkitti
Copy link
Contributor Author

mkitti commented Nov 28, 2022

We implemented the use of AddDllDirectory. SetDefaultDllDirectories created issues with loading other libraries due to Julia's use of LOAD_WITH_ALTERED_SEARCH_PATH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants