__usbstorage_Shutdown in usbstorage.c doesn't release IOS handles

[Kevinn1109]

I am working on a project whereby Homebrew using libogc launches a game mod that does not. In the homebrew code I'm initializing sd and usb so that mods can be loaded from there.

Now I'm adding USB features to the mod, I have found a small oversight in the usbstorage code. Unlike the sd library, usbstorage does not always call IOS_Close for the handles it opened when calling shutdown on the usb disc interface, most noteworthy being a scenario whereby no usb device is plugged in. This causes a conflict with the game that's being launched, since IOS doesn't support more than one usb handle at the time.

I'm aware of the option to call IOS_ReloadIOS to work around this problem, but that shouldn't be required to release all handles opened by libogc modules.

[DacoTaco]

with 'scenario whereby no usb device is plugged in' , do you mean that you mounted it in code and you unplugged it while the code had it mounted?

EDIT : also, is there code we could look at that can reproduce this?

[Kevinn1109]

There is no particular code related to the issue, the issue is that the usb module in Libogc does not provide any way to close the IOS handle. When no usb device is plugged in, calling shutdown even does nothing at all. An example of this being a problem is when using Libfat, since Libfat automatically opens both an sd and usb handle, even when usb is not required. This calls IOS_Open for both devices. In the case of sd, IOS_Close is called in almost all situations when the device is closed, but only in very specific cases is IOS_Close called for the usb handle. So the handle remains open until IOS is reloaded.

I have found out that IOS supports multiple usb handles at the same time in the meantime, which makes this slightly less an issue, but it would still be nice to provide a way to close an IOS handle without full reload.

[DacoTaco]

from what i can tell usbstorage opens the device through USB_Open (which in turn calls IOS_Open) when libfat (correct me if im wrong) or you would call IsInserted and a device is inserted, which can call USBStorage_Open. however, it first checks __vid and __pid to see if they are 0 ( see here ).

__usbstorage_Shutdown does the exact same check , together with a check if the handle is open, before closing the handle using USBStorage_Close. so something must have changed in the state between those 2 moments in time from what i can tell without debugging or creating a test case.
hence me asking if between open(/poll for devices) & close a usb device was inserted/removed.

a reason i could think it would leave a handle open is if it did an usb_open during an operation (read/write/isInserted) without keeping the handle in memory, in which a test case/code would be nice to have to investigate..
you don't have a patch either i assume?

if i find the time i will investigate this in priiloader, but afaik the handle gets closed correctly when it unmounts the usb.

[DacoTaco]

Ive just done a test in priiloader. here are the steps i took :

• mount the USB using __io_usbstorage.startup(), followed by a __io_usbstorage.isInserted() & 'fatMount("fat", &__io_usbstorage,0, 8, 64) )'
• get the handle value (to compare later)
• unmount using fatUnmount("fat:/"); followed by a __io_usbstorage.shutdown();
• remount like before
• get the handle to compare

the handles i got were the same.

however, unplugging and plugging it back in without a proper unmount/shutdown did get me a different handle, probably because my that polls for sd/usb changes doesn't shutdown the interface like when i unmounted it manually.
is this the case you are talking about?

[DacoTaco]

pinging @Kevinn1109

[Kevinn1109]

Thanks for the ping, for some reason I was never notified about your follow-up.

The situation you're describing is indeed the expected behaviour that also should occur in homebrew, since the handle is reused when you attach a second time without calling IOS_Open. The issue begins when a certain homebrew application launches a different application where all stored handles are lost or when homebrew requires manually opening a handle. The handle that would otherwise be reused is lost, and the launched application is unable to open a new handle using the same id unless IOS is reloaded in the meantime. There's no way to force an IOS_Close.

It's an edge case that shouldn't be a true issue when the secondary code does proper checks and tries to IOS_Open using a different index if 0 is already taken, but it would be nice to be able to properly close the handle nevertheless.

[DacoTaco]

Did you also see my investigation above?
USBStorage_Open & USBStorage_Close seem to both open and close the handle correctly, calling USB_Open & USB_CloseDevice respectively if a valid device is detected and a usbstorage_startup & shutdown are executed.

when that certain homebrew application launches a different application without reloading IOS ( are we talking about hbc here?) then that application would need to make sure all handles are closed. i don't think it is libogc's job to keep track of opened handles by a different application.

[DacoTaco]

pinging @Kevinn1109

[Kevinn1109]

Sorry for the very late reply.

Your investigation does indeed show that behind the screens the same handle is used again. But the conclusion you draw from it isn't the correct one. Calling shutdown on the usbstorage does not close the IOS handle, while it remembers to reuse it when you open it again rather than opening a new IOS handle. In regular applications this shouldn't be an issue. The situation I'm describing is an edgecase, but I assume it's not correct in any circumstance that the IOS handle remains open until an IOS_Reload is performed.

[DacoTaco]

ive said it before, a use case/code to show the issue would be helpful for me to investigate your issue.

USBStorage_Close (called from the devops handle) calls USB_Close with the handle, which in turn (if the handle > 0) does an USBV5_CloseDevice ( which cleans up the internal structures for the USB2 code if its still in there). if USBV5_CloseDevice didn't get returned an error, it does an IOS_Close.

the only thing i can see where it could fail is if device is (no longer?) connected to the usb2 host chip and/or already removed from the internal structure, how i don't know. thats where i need the code/use case to figure that out.
if its no longer connected to the usb port , i assume ios has (should?) have cleaned up the handle linked to the device ( i haven't Reverse engineered IOS that far in ) and it should be fine.

you also seem to act like you know where the issue is. so please provide code that shows the issue, a patch or highlight where the issue lies because im not seeing any issues without knowing more from your code/use case/device

[Kevinn1109]

static bool __usbstorage_Shutdown(void)
{
	if (__vid != 0 || __pid != 0)
		USBStorage_Close(&__usbfd);

	return true;
}

This function is called when closing an USB handle. It requires __vid or __pid to be set and doesn't do anything otherwise. The two are set when calling __usbstorage_IsInserted and nowhere else. So this requires IsInserted to be called in order to be able to IOS_Close the handle. This is a point I initially did not know about, hence the confusion.

The IOS_Close itself too is hidden behind an if statement. If this fails, the programmer has no way of knowing the handle is still open, although one would expect that closing a device releases all handles no matter what.

s32 USB_CloseDevice(s32 *fd)
{
	s32 ret = IPC_EINVAL;
	if (fd) {
		ret = USBV5_CloseDevice(*fd);
		if (ret==IPC_EINVAL && *fd>0)
			ret = IOS_Close(*fd);
		if (ret>=0) *fd = -1;
	}

	return ret;
}

In this case, if USBV5_CloseDevice fails, which is possible when the device has changed in the meantime as you pointed out, IOS_Close isn't called, but the homebrew builder has no way of knowing that the close was not successful. I personally think the USB handle should act exactly like its SD counterpart; namely that IOS_Close gets called no matter what if the handle is open. It could be that I'm not 100% aware of some IOS processes preventing the close, but in that case shutdown shouldn't return true.

I think the most noteworthy example is libfat. When using libfat, the usb device is put in a state where it cannot be released.