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

add dll folder as loading library search path #145

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

WeilongWen
Copy link
Contributor

No description provided.

@markaren
Copy link
Member

markaren commented Sep 5, 2024

Just to make sure I got this right:

On Windows, this PR adds the directory of the dll to be loaded to PATH (or akin to it), so that dependent DLLs can be loaded?
Sounds reasonable. How about RemoveDllDirectory, does the directory not need to be searchable for the duration of the dll usage?

@WeilongWen
Copy link
Contributor Author

WeilongWen commented Sep 5, 2024

Yeah, Some of fmu have dependent dlls. I have to add directory of the dll to the path. After the dll is loaded, entry dll is loaded successfully and already in use. RemoveDllDirectory can be release.

@markaren
Copy link
Member

markaren commented Sep 5, 2024

But what if the dependent dlls are loaded dynamically in the first dll? I assume that would cause issues? I suppose a more eloborate solution would be to keep the searchpath as long as the FMU is used.

With that said, I do suppose this resolves one class of issues.

@markaren
Copy link
Member

markaren commented Sep 5, 2024

Possible solution to #125

@WeilongWen
Copy link
Contributor Author

You are right. I'm ignoring that dlls that are being dependent on May also be loaded dynamically. We can keep the search path until the fmu is released.

@markaren
Copy link
Member

markaren commented Sep 6, 2024

I'm merging this as is, as it solves an issue. Keeping the path for a longer duration can be a possible future enhancement.

@markaren markaren merged commit 420d0be into NTNU-IHB:master Sep 6, 2024
@WeilongWen
Copy link
Contributor Author

In the near future, I will do this.

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

Successfully merging this pull request may close these issues.

2 participants