This repository was archived by the owner on Oct 18, 2026. It is now read-only.
Repository navigation
Apply the review fixes from ytai/ioio#466 and #468 - #2
Merged
Merged
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Since the handshake left the lock, a disconnect() between the state check and waitForOpen() was lost: waitForOpen() set shouldTryOpen_ again, and without an accessory the thread waited forever. Check the state and open under one lock; only the handshake runs without it. An instance that was disconnected before waitForConnect() reports a lost connection.
InetAddress.getLoopbackAddress() exists on Android only since API 19, and the library declares minSdkVersion 14: on API 14 to 18, opening the socket bootstrap threw NoSuchMethodError. Build the address from its bytes instead; adbd and "adb forward" connect to 127.0.0.1.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Since the handshake runs without the lock, a cancelled connection A could still be in its handshake while a new connection B opens. The descriptor, the streams and shouldTryOpen_ lived in the bootstrap, shared by all connections: A wrote to B's stream and could take B's handshake reply, B set shouldTryOpen_ again so that A was no longer cancelled, and A's failed handshake closed B's descriptor. Keep the descriptor and the streams in the Connection, and end waitForOpen() and the handshake on that connection's own state. disconnect() closes only that connection's descriptor; the permission request is cancelled when a connection that is still opening gives up, and dropped once the accessory is open.
This was referenced Oct 17, 2026
|
I would recommend to use upstream and fix issues there. I'll approve it for sure |
Member
|
I d love to not having maintain a fork. We will switch once its all upstreamed. |
14 of 17 tasks
lordfolken
pushed a commit
to XCSoar/XCSoar
that referenced
this pull request
Oct 17, 2026
All of the IOIOLib fixes XCSoar needs are now in upstream master: the accessory handshake outside the lock and its connection races (ytai/ioio#466), canClose() (#465), the disconnect deadlock (#469), the USB permission read from UsbManager (#471), the loopback-only socket (#468), the other #470-#474 fixes, and the fallback to interface IOIO0003 for older firmware such as the Androport's (#485). The XCSoar/ioio fork is no longer needed, so point the submodule at ytai/ioio directly. 74c87c6 lacked the race fixes from XCSoar/ioio#2 and the permission fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review on the upstream PRs found three races in the accessory connection and an API-level problem in the socket bootstrap that #1 also carries:
disconnect()between the state check andwaitForOpen()was lost:waitForOpen()setshouldTryOpen_again and, without an accessory, waited forever. The check and the open now hold the lock together; only the handshake runs without it.disconnect()after the handshake but beforeCONNECTEDwas overwritten (commit by @hannesa2).shouldTryOpen_were shared: it could write to the new connection's stream, take its handshake reply or close its descriptor. Each connection now keeps its own descriptor and streams.InetAddress.getLoopbackAddress()is API 19 while the library declares minSdk 14; the address is now built from its bytes.All four compile against android-34 with javac 21. Tested on an IOIO-OTG (SPRK0020, firmware IOIO0506) behind an LXNAV S10 with a Pixel 3a, through XCSoar built with this branch (
66c80028): connect, exit while connected or while connecting, and unplug/replug while running all work; details in ytai#466. Restarting without replugging right after a connected session still hangs in the handshake (Android OpenAccessory bug), but the hang can now be cancelled.