Remove panic during NVML shutdown

[alexbagirov]

We're using go-nvml in production and have concerns about receiving a panic() during NVML shutdown - this can kill other services depending on the library.

This PR removes this panic(), although I'm not sure which approach is better. My proposal is to return both Return and error where Return corresponds to nvml.Init() and nvml.Shutdown() calls, error corresponds to dlOpen() and dlClose().

[alexbagirov]

@klueska Could you please make a review for this PR?

[elezar]

@alexbagirov would implementing SafeInit(), SafeInitWithFlags() and SafeShutdown() functions be feasible? Something along the lines of:

func SafeInit() (ret Return) {
	defer func() {
		if err := recover(); err != nil {
			ret = ERROR_UNKNOWN
		}
	}()
	return Init()
}

We could even make these the default implementations -- although the downside is that we lose the reason for the panic in this case. Adding some global error that can be queried by a client (similar to C.dlerror()) may be useful here.

Note that this kind of wrapper can be implemented at client side without modifying the bindings.

My major concern with modifying the function signatures is that this goes against the intent of having this package be bindings of the NVML API.

[klueska]

I am not in favor of having Init() and Shutdown() return an additional error paramater (as @elezar suggests it goes against the nature of these being 1:1 bindings of the C-based library).

That said, I would be fine doing away with the panic in favor of returning NVML_ERROR_UNKNOWN. Some information is obviously lost there, but I think it will be rare that this ever returns an error anyway.

[elezar]

Since this change was originally proposed there have been a number of changes. We have reworked how the library is represented internally.

I think what I would like to see is extending the Library interface to include a GetErrors() map[string]error or GetLastError() error function that can be called instead of the panic. We would then set the error accordingly instead of panicing. We could add a top-level Option to allow users to revert to the existing behaviour (WithPanicOnShutdownErrors(bool)).

I could make these changes at some point in the future, but my capacity is currently limited. Would you be able to make these changes?

[elezar]

Hi @alexbagirov

Since the basic ask is covered by #107, would you be willing to update this PR to implement something as described in #70 (review)?