Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(usbdevice): close closed channel exception #86

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

Yu-Jack
Copy link
Contributor

@Yu-Jack Yu-Jack commented Jul 31, 2024

Problem:

Reproduce Steps

  1. Plug in USB device A.
  2. Unplug USB device A and plug it in again. This increases the device number.
  3. Create a usbdeviceclaim.
  4. Reboot the host.
  5. Observe that the device number of USB device A is reset to the value from step 1.
  6. Delete the usbdeviceclaim.
  7. Observe the error from pcidevices-controller: close closed channel.

In this cause, the original usbdeviceclaim won't work due to device number changed. That's expected.

However, the plugin.deregistered channel is closed twice due to multiple calls to ListAndWatch. But, I cannot find the root cause yet. Check #86 (comment)

bug.mp4

Solution:

Do not use close, just make it buffered channel.

Related Issue:

harvester/harvester#5762

Test plan:

Follow above steps, it shouldn't complain the same error.

resolved.mp4

Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks, do we need a separate issue for tracking the root cause?

@Yu-Jack
Copy link
Contributor Author

Yu-Jack commented Aug 8, 2024

@ibrokethecloud @WebberHuang1118

I find the root cause, it's quite straightforward.

We're trying to open gRPC server, and close it after error happens. Each time we start the gRPC server, kubelete invokes ListAndWatch function.

But, the program is stuck here since it didn't receive any value from that two channels.

done := false
for !done {
	select {
	case <-plugin.update:
		if err := sendUpdate(plugin.devicesToKubeVirtDevicePlugin()); err != nil {
			return err
		}
	case <-plugin.stop:
		done = true
}

So, I'll add new channel which presents that the gRPC server is closed or not. Please check my latest commits to see the changes, thanks!

@Yu-Jack Yu-Jack force-pushed the usbdevice-fix-close-closed-chan branch from 1575e18 to 0063bac Compare August 8, 2024 08:32
Signed-off-by: Jack Yu <jack.yu@suse.com>
Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

Copy link
Collaborator

@ibrokethecloud ibrokethecloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. thanks.

@Yu-Jack Yu-Jack merged commit a20076c into harvester:master Aug 19, 2024
5 checks passed
@Yu-Jack Yu-Jack deleted the usbdevice-fix-close-closed-chan branch August 19, 2024 03:55
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.

3 participants