WSLA: Update GetExitEvent, ImportImage, LoadImage, and Logs to use system handle marshalling.

[benhillis]

This change updates the WSLA interface to use system handle marshalling when possible. I did not update any of the methods where the handle type can vary (GetStdHandle, Attach, WarningsPipe), but we can consider that in a future change.

[Copilot]

Pull request overview

This PR updates the WSLA interface to use system handle marshalling for better type safety and cleaner code. The changes convert several methods from using ULONG handle representations to native HANDLE types with RPC system handle marshalling attributes.

Changes:

• Updated IDL interface definitions to use system_handle attributes for GetExitEvent, LoadImage, ImportImage, and Logs methods
• Modified implementation code to work directly with HANDLE types instead of converting to/from ULONG
• Added validation tests for null parameter handling in DeleteImage and Logs methods

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/windows/wslaservice/inc/wslaservice.idl	Updated interface definitions to use system_handle marshalling for event, file, and pipe handles
src/windows/wslaservice/exe/WSLASession.h	Updated LoadImage and ImportImage method signatures to use HANDLE instead of ULONG
src/windows/wslaservice/exe/WSLASession.cpp	Removed manual handle conversion code and simplified ImportImageImpl to work directly with HANDLE
src/windows/wslaservice/exe/WSLAProcess.h	Updated GetExitEvent signature to return HANDLE instead of ULONG
src/windows/wslaservice/exe/WSLAProcess.cpp	Simplified GetExitEvent to use helper function instead of manual conversion
src/windows/wslaservice/exe/WSLAContainer.h	Updated Logs method signature to use HANDLE pointers instead of ULONG pointers
src/windows/wslaservice/exe/WSLAContainer.cpp	Simplified Logs implementation to use release() instead of manual handle conversion and duplication
src/windows/wslaservice/exe/ServiceProcessLauncher.cpp	Simplified GetExitEvent to use helper function for handle duplication
src/windows/wsladiag/wsladiag.cpp	Removed unnecessary reinterpret_cast from Logs call
src/windows/common/WSLAProcessLauncher.cpp	Removed unnecessary reinterpret_cast from GetExitEvent call
test/windows/WSLATests.cpp	Updated test code to pass handles directly and added validation tests for null parameter handling

[OneBlue]

I think using handle marshalling for exit events is a good idea, but I'd have the opinion to leave the other ones as ULONG*, so we don't prevent ourselves from changing their types in the future (also some methods can return different types based on the type of process)

[benhillis]

I think using handle marshalling for exit events is a good idea, but I'd have the opinion to leave the other ones as ULONG*, so we don't prevent ourselves from changing their types in the future (also some methods can return different types based on the type of process)

I disagree, I think we should use handle marshalling everywhere possible to avoid strange behavior when the callers specify unexpected handle types. If we decide we need more flexibility we can add it.

[OneBlue]

I disagree, I think we should use handle marshalling everywhere possible to avoid strange behavior when the callers specify unexpected handle types. If we decide we need more flexibility we can add it.

I think this would be OK as long as it doesn't lock us down in the future. If let's say we ship with a sh_pipe in the .IDL, and then in the future we decide that we want to return an hvsocket, will the client COM side accept the new handle type ? If so, I think this is OK

[benhillis]

I disagree, I think we should use handle marshalling everywhere possible to avoid strange behavior when the callers specify unexpected handle types. If we decide we need more flexibility we can add it.

I think this would be OK as long as it doesn't lock us down in the future. If let's say we ship with a sh_pipe in the .IDL, and then in the future we decide that we want to return an hvsocket, will the client COM side accept the new handle type ? If so, I think this is OK

That's a good question. I'd argue that either way that would be an API breaking change.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

[Copilot]

Same issue here: wil::unique_handle should be passed to Logs via an out-parameter helper (e.g., .put()), not by taking its address. Taking &validHandle does not produce a HANDLE* and can break compilation or corrupt the RAII wrapper.

[Copilot]

container->Logs expects HANDLE* out-params; &stdoutLogs/&stderrLogs are wil::unique_handle* and are not a safe/valid substitute. Use the WIL out-parameter helper (e.g., stdoutLogs.put() / stderrLogs.put()) to populate the RAII handle.

Suggested change

	THROW_IF_FAILED(container->Logs(flags, &stdoutLogs, &stderrLogs, 0, 0, 0));
	THROW_IF_FAILED(container->Logs(flags, stdoutLogs.put(), stderrLogs.put(), 0, 0, 0));

[Copilot]

GetExitEvent now takes a HANDLE* out-param, but &event is a wil::unique_event* (not a HANDLE*). Use the appropriate WIL out-parameter helper (e.g., event.put()) so the returned handle is stored correctly in the RAII wrapper.

Suggested change

	THROW_IF_FAILED(m_process->GetExitEvent(&event));
	THROW_IF_FAILED(m_process->GetExitEvent(event.put()));

[Copilot]

container.Logs expects HANDLE* out-params, but passing &stdoutLogs/&stderrLogs provides a wil::unique_handle*. This is either a compile-time type mismatch or relies on undefined layout/overloads. Use the WIL out-parameter helper (e.g., stdoutLogs.put() / stderrLogs.put()) so the handle value is written to the internal HANDLE storage safely (and any prior value would be closed).

Suggested change

	VERIFY_SUCCEEDED(container.Logs(Flags, &stdoutLogs, &stderrLogs, Since, Until, Tail));
	VERIFY_SUCCEEDED(container.Logs(Flags, stdoutLogs.put(), stderrLogs.put(), Since, Until, Tail));