Skip to content

fix: adjust error handling in suspend script#304

Closed
ben16w wants to merge 0 commit intoLoveRetro:mainfrom
ben16w:main
Closed

fix: adjust error handling in suspend script#304
ben16w wants to merge 0 commit intoLoveRetro:mainfrom
ben16w:main

Conversation

@ben16w
Copy link

@ben16w ben16w commented May 12, 2025

This PR updates the suspend script with these changes:

  • Removed -o pipefail from the set command as it's not supported in sh.
  • Fixed backgrounding of udhcpc and after functions by replacing unnecessary double parentheses with standard backgrounding (&).
  • Made writing to /sys/power/state non-fatal by appending || true, preventing the script from exiting if the suspend command fails.

I've made these changes because when developing minui-power-control sometimes the WiFi doesn't restore after resuming from suspend. Looking at the logs, I found:

Preparing for suspend...
Saving mixer state...
Blocking wireless...
Suspending...
sh: write error: Device or resource busy

The suspend script is failing at echo mem >/sys/power/state (even though the device does still suspend). This causes the script to fail and exit. The after function then doesn't run and WiFi/Bluetooth are not restored. This PR fixes that so that the after function always runs regardless of erorrs with echo mem >/sys/power/state.

@frysee
Copy link
Member

frysee commented May 12, 2025

Interesting, why would suspend fail? Arent we calling that in a loop until it succeeds?

@ben16w
Copy link
Author

ben16w commented May 12, 2025

echo mem >/sys/power/state only gets called once, not in any sort of loop. Tbh not sure why it's failing, I'll have a look at dmesg. I do feel that WiFi/Bluetooth should be restored regardless of the outcome of the suspend

@ben16w
Copy link
Author

ben16w commented May 12, 2025

Did a quick Google search. It looks pretty command and the error can be ignored if it's still suspending successfully. Basically, the "Device or resource busy" error is reported by the kernel when some kernel driver or subsystem says that it is not ready for suspend. The system still suspends if the kernel determines it's not critical enough.

@ben16w
Copy link
Author

ben16w commented May 12, 2025

I had a look at dmesg if anything was reporting busy with "dmesg | grep -i busy" and nothing jumped out at me, but I'm pretty sure that's what's causing it.

@frysee
Copy link
Member

frysee commented May 12, 2025

echo mem >/sys/power/state only gets called once, not in any sort of loop. Tbh not sure why it's failing, I'll have a look at dmesg. I do feel that WiFi/Bluetooth should be restored regardless of the outcome of the suspend

I was thinking of this piece of code: https://github.com/LoveRetro/NextUI/blob/main/workspace/all/common/api.c#L2883

@frysee
Copy link
Member

frysee commented May 12, 2025

I had a look at dmesg if anything was reporting busy with "dmesg | grep -i busy" and nothing jumped out at me, but I'm pretty sure that's what's causing it.

I'm wondering if this is something that needs some attention and if its related to the limbo/poweroff bug in any way. Not in this ticket, of course.

@ben16w
Copy link
Author

ben16w commented May 12, 2025

Tbh something like that could well be blocking poweroff. In terms of that bit of code is that calling the suspend script multiple times? If so then the current state of WiFi/Bluetooth will be lost because of the same problem (i.e. if there's any error then WiFi/Bluetooth isn't restored)

@ben16w
Copy link
Author

ben16w commented May 12, 2025

Instead, it might make more sense to put the code in the after function in a cleanup function that is always run, regardless of how the script exits. Then api.c can just keep on running it until it succeeds. eg:

trap "cleanup" EXIT INT TERM HUP QUIT

cleanup() {
	>&2 echo "Restoring mixer state..."
	alsactl --file "$asound_state_dir/asound.state.post" store || true
	alsactl --file "$asound_state_dir/asound.state.pre" restore || true

	>&2 echo "Unblocking wireless..."
	echo 1 >/sys/class/rfkill/rfkill0/state || true
...

@frysee
Copy link
Member

frysee commented May 14, 2025

Are you bringing this one back? Currently trying to figure out why CI fails on it.

@ben16w
Copy link
Author

ben16w commented May 14, 2025

I just forgot to make my changes in a branch so I couldn't pull from upstream. Going to open a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants