Skip to content

Harden the USB adapter lifecycle #116

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.

Sign up for GitHub

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

Open
iflyhere wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Harden the USB adapter lifecycle #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ public Map<String, UsbDevice> getAttachedAdapters() {

public synchronized void refreshAdapters() {
Map<String, UsbDevice> attachedAdapters = getAttachedAdapters();
if (attachedAdapters == null) {
Log.e(TAG, "Could not read the usb device filter, skipping adapter refresh.");
return;
}

boolean missingPermissions = false;
android.hardware.usb.UsbManager usbManager =
Expand All @@ -128,8 +132,13 @@ public synchronized void refreshAdapters() {
if (!usbManager.hasPermission(entry.getValue())) {
binding.tvMessage.setVisibility(View.VISIBLE);
binding.tvMessage.setText("No permission for wifi adapter(s) " + entry.getValue().getDeviceName());
// Android 14 refuses to deliver a PendingIntent built from an implicit
// intent to a runtime registered receiver, so the permission result never
// arrives unless the package is set explicitly.
Intent permissionIntent = new Intent(WfbLinkManager.ACTION_USB_PERMISSION);
permissionIntent.setPackage(context.getPackageName());
PendingIntent pendingIntent = PendingIntent.getBroadcast(context, 0,
new Intent(WfbLinkManager.ACTION_USB_PERMISSION), PendingIntent.FLAG_IMMUTABLE);
permissionIntent, PendingIntent.FLAG_IMMUTABLE);
usbManager.requestPermission(entry.getValue(), pendingIntent);
missingPermissions = true;
}
Expand All @@ -155,8 +164,11 @@ public synchronized void refreshAdapters() {
if (activeWifiAdapters.containsKey(entry.getKey())) {
continue;
}
startAdapter(entry.getValue());
activeWifiAdapters.put(entry.getKey(), entry.getValue());
// Only track it as active if it actually came up, otherwise a failed adapter
// is never retried on the next refresh.
if (startAdapter(entry.getValue())) {
activeWifiAdapters.put(entry.getKey(), entry.getValue());
}
}

if (activeWifiAdapters.isEmpty()) {
Expand Down Expand Up @@ -204,7 +216,10 @@ public synchronized boolean startAdapter(UsbDevice dev) {
String text = "Starting wfb-ng channel " + wifiChannel + " with " + String.format(
"[%04X", dev.getVendorId()) + ":" + String.format("%04X]", dev.getProductId());
binding.tvMessage.setText(text);
wfbLink.start(wifiChannel, bandWidth.getValue(), dev);
if (!wfbLink.start(wifiChannel, bandWidth.getValue(), dev)) {
binding.tvMessage.setText("Could not open wifi adapter " + dev.getDeviceName());
return false;
}
return true;
}
}
11 changes: 3 additions & 8 deletions app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@
#undef TAG
#define TAG "pixelpilot"

#define CRASH() \
do { \
int *i = 0; \
*i = 42; \
} while (0)

std::string generate_random_string(size_t length) {
const std::string characters = "abcdefghijklmnopqrstuvwxyz";
std::random_device rd;
Expand Down Expand Up @@ -283,8 +277,9 @@ int WfbngLink::run(JNIEnv *env, jobject context, jint wifiChannel, jint bw, jint

void WfbngLink::stop(JNIEnv *env, jobject context, jint fd) {
if (rtl_devices.find(fd) == rtl_devices.end()) {
__android_log_print(ANDROID_LOG_ERROR, TAG, "rtl_devices.find(%d) == rtl_devices.end()", fd);
CRASH();
// Happens when the adapter was already gone by the time the stop arrived, e.g. it
// was unplugged or the hub re-enumerated it. Nothing left to stop.
__android_log_print(ANDROID_LOG_WARN, TAG, "stop: no rtl device for fd=%d, already gone", fd);
return;
Comment on lines +282 to 283

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Early detach wedges worker 🐞 Bug ☼ Reliability

If stop() runs after Java starts the worker but before native run() registers the fd, the new
early return sends no stop signal and Java then blocks indefinitely in Thread.join(). The worker
can subsequently finish initialization and enter the blocking RX loop, wedging detach handling or
activity shutdown.
Agent Prompt 
## Issue description
A stop request arriving before native fd registration is discarded, after which Java can wait forever for the worker that proceeds into its blocking RX loop.

## Issue Context
`WfbNgLink.start()` publishes and starts the Java thread asynchronously. Native setup does not insert the fd into `rtl_devices` until later, while Java `stop()` immediately calls native stop and then joins without a timeout.

## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[86-128]
- app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[278-283]
- app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java[111-115]
- app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java[149-157]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
auto dev = rtl_devices.at(fd).get();
Expand Down
30 changes: 27 additions & 3 deletions app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,35 @@ public void nativeSetUseStbc(int use) {
nativeSetUseStbc(nativeWfbngLink, use);
}

public synchronized void start(int wifiChannel, int bandWidth, UsbDevice usbDevice) {
public synchronized boolean start(int wifiChannel, int bandWidth, UsbDevice usbDevice) {
Log.d(TAG, "wfb-ng monitoring on " + usbDevice.getDeviceName() + " using wifi channel " + wifiChannel);
UsbManager usbManager = (UsbManager) context.getSystemService(Context.USB_SERVICE);
// Returns null when the permission was revoked or the device disappeared between
// the permission check and here, which is easy to hit on a re-enumerating hub.
UsbDeviceConnection usbDeviceConnection = usbManager.openDevice(usbDevice);
if (usbDeviceConnection == null) {
Log.e(TAG, "Could not open " + usbDevice.getDeviceName() + " (no permission or already gone)");
return false;
}
int fd = usbDeviceConnection.getFileDescriptor();
if (fd < 0) {
Log.e(TAG, "Invalid file descriptor for " + usbDevice.getDeviceName());
usbDeviceConnection.close();
return false;
}
Thread t = new Thread(() -> nativeRun(nativeWfbngLink, context, wifiChannel, bandWidth, fd));
t.setName("wfb-" + usbDevice.getDeviceName().split("/dev/bus/usb/")[1]);
t.setName(threadNameFor(usbDevice));
linkThreads.put(usbDevice, t);
linkConns.put(usbDevice, usbDeviceConnection);
linkThreads.get(usbDevice).start();
t.start();
Log.d(TAG, "wfb-ng thread on " + usbDevice.getDeviceName() + " started.");
return true;
}

private static String threadNameFor(UsbDevice usbDevice) {
String name = usbDevice.getDeviceName();
String[] parts = name.split("/dev/bus/usb/");
return "wfb-" + (parts.length > 1 ? parts[1] : name);
}

public synchronized void stopAll() throws InterruptedException {
Expand All @@ -114,9 +132,13 @@ public synchronized void stopAll() throws InterruptedException {
if (t != null) {
t.join();
}
// The connection holds a dup of the usbfs fd. Without close() every
// attach/detach cycle leaks one, until the process runs out.
entry.getValue().close();
Log.d(TAG, "wfb-ng thread on " + entry.getKey().getDeviceName() + " done.");
}
linkThreads.clear();
linkConns.clear();
}

public synchronized void stop(UsbDevice dev) throws InterruptedException {
Expand All @@ -131,6 +153,8 @@ public synchronized void stop(UsbDevice dev) throws InterruptedException {
t.join();
}
linkThreads.remove(dev);
linkConns.remove(dev);
conn.close();
}

public void SetWfbNGStatsChanged(final WfbNGStatsChanged callback) {
Expand Down