Resource leaks in online compiler implementation

[AlexeySachkov]

llvm/sycl/source/detail/online_compiler/online_compiler.cpp

Lines 101 to 104 in 83fe1c1

	void *OclocLibrary = sycl::detail::ur::loadOsLibrary(OclocLibraryName);
	if (!OclocLibrary)
	throw online_compile_error("Cannot load ocloc library: " +
	OclocLibraryName);

loadOsLibrary is a wrapper around dlopen/LoadLibraryEx and both those functions require a de-allocation call once resource has been used. However, we don't do that at all in our online compiler implementation.

Note that resource leak will happen every time we throw an exception. It is not like this is a high-impact thing, because if we threw an exception saying that we couldn't load ocloc, it is unlikely that we will succeed on retry and most likely such exception will cause a SYCL app to reach a full stop. At this point, OS should help us properly free all resources.

However, that is still an issue which static analyzer highlights, regardless of its actual impact. The suggestion here is that we update our loadOsLibrary function to return a unique_ptr which will have a correct deleter assigned to it. That way, resources will be freed automatically.

[uditagarwal97]

@KseniyaTikhomirova, FYI. I think your PR #16517 will address this issue as well.

[KseniyaTikhomirova]

@KseniyaTikhomirova, FYI. I think your PR #16517 will address this issue as well.

no, I didn't fix online compiler impl. There are 3 similar places with leak, I fixed two of them which are not deprecated, online_compiler is deprecated so I left it as it is for now.

[uditagarwal97]

@KseniyaTikhomirova, FYI. I think your PR #16517 will address this issue as well.

no, I didn't fix online compiler impl. There are 3 similar places with leak, I fixed two of them which are not deprecated, online_compiler is deprecated so I left it as it is for now.

I see. Do we plan to fix the leak in online_compiler in future, given that it's depreciated? If not, I think we can close this issue as well along with your PR.

[KseniyaTikhomirova]

@KseniyaTikhomirova, FYI. I think your PR #16517 will address this issue as well.

no, I didn't fix online compiler impl. There are 3 similar places with leak, I fixed two of them which are not deprecated, online_compiler is deprecated so I left it as it is for now.

I see. Do we plan to fix the leak in online_compiler in future, given that it's depreciated? If not, I think we can close this issue as well along with your PR.

Hi, I clarified how mem leak in deprecated class should be properly handled and I think that we will keep this issue open till the online_compiler class is removed completely.

[cperkinsintel]

The experimental online_compiler extension has been marked as deprecated for over a year and with #16776 it has now been removed. Its functionality is better provided by the kernel_compiler extension.