Build config off the main thread; register tuic/juicity import schemes#118
Merged
Conversation
…rt schemes The connect path ran proxy.init() -> buildConfig() (synchronous group/profile DAO reads) inside runOnMainDispatcher, so with the main-thread-DB allowance removed in debug (Plan 027) starting a profile threw 'Cannot access database on the main thread' and the service failed to start. Wrap proxy.init() in onDefaultDispatcher so the config build and its DAO reads run off the UI thread; the surrounding notification/state/UI calls stay on main. This is the main-thread site the 027 flag was designed to surface (device-caught on a real connect). Also register the tuic and juicity schemes in the profile-import VIEW intent-filter for parity with the other protocols (the parser in Formats.kt already handles both; only the manifest deep-link entry was missing, so they previously imported only via QR/paste).
📝 WalkthroughWalkthroughThe PR adds new profile import URI schemes and moves selector reload, proxy initialization, and proxy teardown work onto background dispatchers. ChangesManifest and service lifecycle updates
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… thread Device StrictMode (allowance off) surfaced three more main-thread DAO sites beyond buildConfig: - ProxyInstance.close() ran the final traffic flush (persist -> updateTraffic / addLifetimeTraffic) via runBlocking on the main thread during teardown; dispatch it on Dispatchers.Default so the DAO writes run off the UI thread. - The connect block called ServiceNotification.genTitle(profile) (a groupDao read) on the main dispatcher; reuse proxy.displayProfileName, which is already computed off-main at ProxyInstance construction. - reloadInner() did canReloadSelector()/getById on the main thread; resolve the selector fast-path tag off-main in the caller (resolveSelectorReloadTag) and pass it in, so reloadInner touches no DB on the UI thread. TUIC verified end-to-end on device after these: service starts, egress flows through the tuic outbound, no 'Cannot access database on the main thread'.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt (1)
551-560: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove
ProxyInstanceconstruction off the main dispatcher
ProxyInstance(profile, this)is created insideonMainDispatcher { ... }, sodisplayProfileName = ServiceNotification.genTitle(profile)still runs on the main thread. WhenDataStore.showGroupInNotificationis enabled, that can still perform agroupDaoread on the UI thread and crash with the main-thread DB check removed. Compute the title before entering the main block, or construct the instance on a background dispatcher.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt` around lines 551 - 560, `ProxyInstance` is still being constructed inside the `onMainDispatcher` block, so `ServiceNotification.genTitle(profile)` can trigger a `groupDao` read on the UI thread when `DataStore.showGroupInNotification` is enabled. Move the `ProxyInstance(profile, this)` construction, or at least the title computation used by `displayProfileName`, to a background dispatcher before `onMainDispatcher`, and keep the main block limited to UI-only work like `createNotification(...)`, `Executable.killAll()`, and `preInit()`.
🧹 Nitpick comments (1)
app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt (1)
74-81: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueEffective for StrictMode, but note the main thread is still blocked.
close()runs on the main thread (viakillProcesses()insidestopRunner'srunOnMainDispatcher). PassingDispatchers.Defaultcorrectly executeslooper.stop()'s synchronous DAO writes on a background thread (satisfying StrictMode), butrunBlockingstill parks the main thread until the flush completes. Iflooper.stop()can be slow, this remains an ANR risk on teardown. Consider bounding it (e.g.withTimeoutOrNull) or making teardown fully async as inpersistStats.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt` around lines 74 - 81, `ProxyInstance.close()` still blocks the main thread because it wraps `looper?.stop()` in `runBlocking(Dispatchers.Default)`, so update this teardown path to avoid an unbounded wait while keeping the DAO flush off the UI thread. Use the existing `looper`/`looper.stop()` flow in `ProxyInstance` and either bound the blocking work with a timeout or convert the shutdown to a fully async pattern like `persistStats`, then clear `looper` only after the stop completes safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt`:
- Around line 551-560: `ProxyInstance` is still being constructed inside the
`onMainDispatcher` block, so `ServiceNotification.genTitle(profile)` can trigger
a `groupDao` read on the UI thread when `DataStore.showGroupInNotification` is
enabled. Move the `ProxyInstance(profile, this)` construction, or at least the
title computation used by `displayProfileName`, to a background dispatcher
before `onMainDispatcher`, and keep the main block limited to UI-only work like
`createNotification(...)`, `Executable.killAll()`, and `preInit()`.
---
Nitpick comments:
In `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt`:
- Around line 74-81: `ProxyInstance.close()` still blocks the main thread
because it wraps `looper?.stop()` in `runBlocking(Dispatchers.Default)`, so
update this teardown path to avoid an unbounded wait while keeping the DAO flush
off the UI thread. Use the existing `looper`/`looper.stop()` flow in
`ProxyInstance` and either bound the blocking work with a timeout or convert the
shutdown to a fully async pattern like `persistStats`, then clear `looper` only
after the stop completes safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c065241d-fc5c-4074-b707-0b7a39b76e11
📒 Files selected for processing (2)
app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.ktapp/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt
Greptile: Formats.kt also parses hysteria2/hy2/vless/anytls/snell but they were missing from the profile-import intent-filter, so sharing those links from another app didn't offer NekoBox. Add them alongside the tuic/juicity entries for full parser<->manifest parity.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Follow-up to #117. Device testing surfaced the main-thread-DB site the Plan 027 flag was designed to catch: the connect path built the config (synchronous group/profile DAO reads via
buildConfig) insiderunOnMainDispatcher, so with the allowance off in debug, starting a profile threwCannot access database on the main threadand the service failed to start.Fixes
BaseService: runproxy.init()(which callsbuildConfig->groupDao.getByIdand other DAO reads) ononDefaultDispatcherinstead of the main dispatcher; notification/state/UI calls stay on main.init()is suspend and touches no UI.AndroidManifest: addtuicandjuicityto the profile-import VIEW intent-filter for parity (theFormats.ktparser already handles both; only the manifest deep-link entry was missing, so they imported only via QR/paste before).Testing
Greptile Summary
This PR fixes a main-thread database access crash introduced by Plan 027 (which removes the main-thread-DB allowance in debug builds):
proxy.init()— and thebuildConfig/ DAO reads it calls — is now dispatched toonDefaultDispatcherinsideonStartConnect, and theTrafficLooper.stop()DB flush inProxyInstance.close()is moved toDispatchers.DefaultviarunBlocking. It also completes the profile-import intent-filter by registeringtuic,juicity,hysteria2,hy2,vless,anytls, andsnellURI schemes thatFormats.ktalready handled but the manifest was missing.BaseService.kt:proxy.init()is wrapped inonDefaultDispatcher { }, keeping all UI/state calls on Main;resolveSelectorReloadTag()is extracted to perform the selector DAO reads off-Main beforereloadInner()is posted to Main.ProxyInstance.kt:runBlockinginclose()gainsDispatchers.Defaultso the final traffic-stat flush does not touch the DB on the Main thread.AndroidManifest.xml: Seven missing URI schemes are added to the VIEW intent-filter, giving them first-class deep-link import on par with QR/paste.Confidence Score: 5/5
The changes are safe to merge — the dispatcher refactoring is correct and the manifest additions are straightforward.
The core fix (moving proxy.init() to onDefaultDispatcher and looper.stop() to Dispatchers.Default) correctly targets the DB-on-main crash without introducing regressions. The selector reload path still re-verifies state on the Main dispatcher before acting, so stale off-thread reads are benign. The only inaccuracy is a comment claiming displayProfileName is computed "off the main thread" when ProxyInstance is still constructed on Main — worth fixing for clarity but does not affect runtime behavior beyond the pre-existing genTitle/showGroupInNotification case.
BaseService.kt — the misleading comment on line 552 and the residual on-main genTitle call (when showGroupInNotification is enabled) are worth a follow-up cleanup pass.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Main as Main Thread participant Default as Default Dispatcher participant DB as SagerDatabase Main->>Default: runOnDefaultDispatcher Default->>DB: proxyDao.getById(selectedProxy) DB-->>Default: profile Default->>Main: onMainDispatcher / onStartConnect Main->>Main: ProxyInstance constructor + genTitle Main->>Main: runOnMainDispatcher - preInit Main->>Default: onDefaultDispatcher - proxy.init Default->>DB: buildConfig groupDao/proxyDao reads DB-->>Default: config built Default-->>Main: resume Main->>Main: startProcesses changeState Connected Note over Main,Default: On stop Main->>Default: runBlocking Dispatchers.Default looper.stop Default->>DB: traffic stat flush DB-->>Default: done Default-->>Main: unblock Note over Main,Default: reload path Default->>DB: resolveSelectorReloadTag proxyDao groupDao DB-->>Default: selectorTag Default->>Main: onMainDispatcher reloadInner Main->>Main: box.selectOutbound(selectorTag)%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Main as Main Thread participant Default as Default Dispatcher participant DB as SagerDatabase Main->>Default: runOnDefaultDispatcher Default->>DB: proxyDao.getById(selectedProxy) DB-->>Default: profile Default->>Main: onMainDispatcher / onStartConnect Main->>Main: ProxyInstance constructor + genTitle Main->>Main: runOnMainDispatcher - preInit Main->>Default: onDefaultDispatcher - proxy.init Default->>DB: buildConfig groupDao/proxyDao reads DB-->>Default: config built Default-->>Main: resume Main->>Main: startProcesses changeState Connected Note over Main,Default: On stop Main->>Default: runBlocking Dispatchers.Default looper.stop Default->>DB: traffic stat flush DB-->>Default: done Default-->>Main: unblock Note over Main,Default: reload path Default->>DB: resolveSelectorReloadTag proxyDao groupDao DB-->>Default: selectorTag Default->>Main: onMainDispatcher reloadInner Main->>Main: box.selectOutbound(selectorTag)Reviews (3): Last reviewed commit: "fix(manifest): register all parsed impor..." | Re-trigger Greptile