Improve dlerror message reporting

[ihnorton]

This PR replaces #7940. The goal is to improve error reporting for dlopen errors, as since #10832 we free the dlerror message in all code paths, before throwing a (Julia) error. Thus we only show no error even after failures.

(whereas before we almost exclusively showed the frequently-incorrect no such file message, but with a rare useful error message if the failure was on the very last file tried).

• for absolute paths, show the error message if file exists.
  this way users always get diagnostics when they try a known
  path.
• for DL_LOAD_PATH/DL_EXT permutations: if a file exists, show
  error. these paths should be complete by construction (i.o.w.
  includes extension). if dlopen fails and the file exists,
  best to let the user know.
• when throwing an error, first print the dlerror, then free the
  handle, then throw the error. this is not ideal, but is the
  simplest way I can think of to actually show the dlerror without
  leaking the errmsg.
• free the handle before throwing an error on Windows

also add some comments.

[ihnorton]

for absolute paths, show the error message if file exists. this way users always get diagnostics when they try a known path.

I think this is the most important change, and should be uncontroversial. Will help greatly in situations like #11374 where the user gives a known-good absolute path and needs to see the real error message.

I think the other changes are correct and hopefully an improvement, but last time we messed with this code it broke a bunch of stuff so I'm counting on a thorough review...

(cc @garrison to make sure I didn't inadvertently re-introduce any leaks)

[quinnj]

@ihnorton, I'm guessing this will also fix #7159?

[ihnorton]

@quinnj thanks for the pointer. Maybe, but I doubt it. That kind of looks like some freaky i/o thing to me because part of the message after ERROR: should always come from our C code.

[garrison]

I didn't comment, but this looked good to me.

This got me curious why the second argument to jl_stat isn't a uv_stat_t*. Seems strange to have to cast it to char* all the time. (git grep jl_stat\( does not provide any insight.)

[vtjnash]

It's incorrect for dlopen to bail on a valid file that can't be used with dlopen. It must finish the search before giving up entirely.

At this point we should really just drop our pretensions of using the uv_lib API and just fully embrace that we have been wrapping dlopen directly.

[ihnorton]

It's incorrect for dlopen to bail on a valid file that can't be used with dlopen. It must finish the search before giving up entirely.

We're only talking about paths here (either absolute, or by construction), not the fallback case. But there are some problematic cases such as:

• user passes /path/to/foo: both /path/to/foo and /path/to/foo.so exist, and the latter is good -> error, because we try /path/to/foo first and dlopen fails. (nvm: this is covered by the absolute path case, and system dlopen will disambiguate)
• user passes "foo": /path/to/foo.so exists and is dl-loadable, /path/to/some/foo exists and is not dl-loadable. If the user puts /path/to/some in DL_LOAD_PATH before /path/to then we will first attempt to load /path/to/some/foo and incorrectly give an error.

The first could be handled by always trying the the +extension version first. The second could be handled by removing the blank extension case so that we only ever try dlopen on paths containing a valid extension. But maybe this is too restrictive?

At this point we should really just drop our pretensions of using the uv_lib API and just fully embrace that we have been wrapping dlopen directly.

Agreed. Might mean a little bit more work on Windows, but seems like a good idea overall. I don't think that would affect the question of dlerror reporting, though.

[ihnorton]

I guess I'm coming around to your suggestion in the other thread. If we fully emulate the dlopen search order (trying DL_LOAD_PATHs first, then LD_LIBRARY_PATHs) and then only ever make one system dlopen call then this whole question of when/whether to report dlerrors goes away.