ADFA-3829 Clear StrictMode violations on create-init flow#1277
Open
hal-eisen-adfa wants to merge 1 commit intostagefrom
Open
ADFA-3829 Clear StrictMode violations on create-init flow#1277hal-eisen-adfa wants to merge 1 commit intostagefrom
hal-eisen-adfa wants to merge 1 commit intostagefrom
Conversation
Address 33 still-reproducible violations from the create-new-project log
(the other 16 from legacy noActivity/emptyActivity/basicActivity templates
were already removed by ADFA-3381).
Off-thread the heavy work:
- TemplateListFragment.reloadTemplates: wrap ITemplateProvider.getInstance
+ getTemplates in withContext(Dispatchers.IO); pre-warm
ITemplateWidgetViewProvider.getInstance so per-bind getInstance is a
cache hit.
- TemplateDetailsFragment.bindWithTemplate: invoke each parameter
beforeCreateView on Dispatchers.IO before constructing the adapter so
that getNewProjectName's File.exists loop runs off the UI thread.
- Parameter.beforeCreateView: now one-shot per instance (reset on
release) so the bind-time call is a no-op once pre-warmed.
- TemplateWidgetViewProviderImpl.createTextField: per-keystroke
ConstraintVerifier.verify (which stats the filesystem for
EXISTS / FILE / DIRECTORY constraints) is now debounced and dispatched
to IO via the view tree's lifecycleScope.
- TemplateWidgetsListAdapter: lift ITemplateWidgetViewProvider.getInstance
out of onBindViewHolder into the adapter's init.
Defer initialization:
- JavaDebugAdapter.vmm: by lazy { Bootstrap.virtualMachineManager() }
eliminates a 507ms NetworkViolation and 4 disk reads at field init.
Tag the JDWP listener socket:
- ListenerState.startListening: wrap with TrafficStats.setThreadStatsTag
/ clearThreadStatsTag to clear UntaggedSocketViolation.
Whitelist vendor-framework call (not app-owned):
- MIUI's NotificationManager.notify lazily inits EpFrameworkFactory via
File.exists; rule + test added.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughRelease NotesPerformance & Thread Safety Improvements:
Strict Mode Compliance:
Risks & Best Practices Considerations:
WalkthroughThis PR introduces asynchronous template widget initialization with debounced input validation, adds a strict-mode whitelist rule for MIUI enterprise JAR checks, and applies performance optimizations to debug initialization and network traffic accounting. ChangesTemplate Async Initialization & Debounced Validation
Strict Mode MIUI Enterprise Whitelist
Performance Optimizations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/app/strictmode/WhitelistEngine.kt`:
- Around line 256-260: The current matchAdjacentFrames invocation that looks for
classAndMethod("java.io.File","exists") followed by the three MIUI frames should
be narrowed to require the downstream NotificationManager call path as well;
update the matcher that builds this rule (the matchAdjacentFrames call) to also
require, in order, a classAndMethod("android.app.NotificationManager","notify")
or classAndMethod("android.app.NotificationManager","notifyAsUser") frame after
(or before, as appropriate to call order) the MIUI EpFrameworkFactory frames so
the rule only triggers when the MIUI chain is part of a notification delivery
path.
In
`@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`:
- Around line 173-183: The coroutine launched in
viewLifecycleOwner.lifecycleScope.launch can overlap older runs and assign the
wrong adapter; add a Job field (e.g., widgetsBindJob) on the fragment, cancel
widgetsBindJob?.cancel() before starting a new
viewLifecycleOwner.lifecycleScope.launch, store the returned Job in
widgetsBindJob, do the Dispatchers.IO work (template.widgets.forEach /
ParameterWidget.beforeCreateView) inside that coroutine, and only after the job
is the active one assign binding.widgets.adapter =
TemplateWidgetsListAdapter(template.widgets) (also keep the existing _binding
null-check) so stale/cancelled jobs cannot overwrite the adapter.
In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt`:
- Around line 125-166: The reloadTemplates coroutine can run multiple concurrent
jobs and older ones may overwrite newer UI state; add a cancellable Job property
(e.g., private var reloadJob: Job? = null) on TemplateListFragment, cancel any
existing reloadJob before starting a new
viewLifecycleOwner.lifecycleScope.launch in reloadTemplates, then assign the
launched Job to reloadJob so only the latest job updates
adapter/binding/warnings; keep the existing _binding check and UI update logic
unchanged.
In
`@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kt`:
- Around line 48-53: Preserve and restore any existing thread stats tag around
the JDWP listener setup: before calling
TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG) read and save the
current tag with TrafficStats.getThreadStatsTag(), then call
TrafficStats.setThreadStatsTag(...), run connector.startListening(args) and in
the finally block restore the saved tag by calling
TrafficStats.setThreadStatsTag(savedTag) instead of
TrafficStats.clearThreadStatsTag(); reference the
TrafficStats.getThreadStatsTag, TrafficStats.setThreadStatsTag,
TrafficStats.clearThreadStatsTag calls and the connector.startListening(args)
invocation in ListenerState.kt to implement this change.
In `@templates-api/src/main/java/com/itsaky/androidide/templates/parameters.kt`:
- Around line 189-203: The one-shot guard in beforeCreateView currently does a
non-atomic check/set on beforeCreateViewInvoked allowing races; make the
check-and-set atomic (e.g., replace the boolean with an AtomicBoolean and use
compareAndSet, or protect the check/set and invocation with a synchronized
block) so that beforeCreateViewInvoked is set and actionBeforeCreateView is
invoked exactly once across threads; update the beforeCreateView method to
atomically test-and-set before invoking actionBeforeCreateView(this).
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt`:
- Around line 179-190: The validation branch toggles root.isErrorEnabled and
only sets root.error when err != null, leaving stale error text when validation
passes; in both the lifecycle-owner branch and the fallback
(ConstraintVerifier.verify) branch inside TemplateWidgetViewProviderImpl
(references: root, root.isErrorEnabled, root.error,
ConstraintVerifier.verify(value, constraints = param.constraints), value,
param.constraints) ensure that when err == null you explicitly clear the error
by setting root.error = null and set root.isErrorEnabled = false so no stale
error text remains after successful validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0c09bb1-f570-4b8d-9de5-78aa7da5a6c0
📒 Files selected for processing (9)
app/src/androidTest/kotlin/com/itsaky/androidide/app/strictmode/WhitelistRulesTest.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateWidgetsListAdapter.ktapp/src/main/java/com/itsaky/androidide/app/strictmode/WhitelistEngine.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kttemplates-api/src/main/java/com/itsaky/androidide/templates/parameters.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt
Comment on lines
+256
to
+260
| matchAdjacentFrames( | ||
| classAndMethod("java.io.File", "exists"), | ||
| classAndMethod("miui.enterprise.EpFrameworkFactory", "isEnterpriseJarExists"), | ||
| classAndMethod("miui.enterprise.EpFrameworkFactory", "get"), | ||
| ) |
Contributor
There was a problem hiding this comment.
Tighten this rule to the NotificationManager call path.
Right now this matcher allows any stack containing the 3-frame MIUI chain, even if it is not triggered from notification delivery. Narrow it by also requiring the downstream NotificationManager.notify/notifyAsUser anchor in order.
Suggested narrowing
- matchAdjacentFrames(
- classAndMethod("java.io.File", "exists"),
- classAndMethod("miui.enterprise.EpFrameworkFactory", "isEnterpriseJarExists"),
- classAndMethod("miui.enterprise.EpFrameworkFactory", "get"),
- )
+ matchAdjacentFramesInOrder(
+ listOf(
+ listOf(
+ classAndMethod("java.io.File", "exists"),
+ classAndMethod("miui.enterprise.EpFrameworkFactory", "isEnterpriseJarExists"),
+ classAndMethod("miui.enterprise.EpFrameworkFactory", "get"),
+ ),
+ listOf(
+ anyOf(
+ classAndMethod("android.app.NotificationManager", "notifyAsUser"),
+ classAndMethod("android.app.NotificationManager", "notify"),
+ ),
+ ),
+ ),
+ )🤖 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/com/itsaky/androidide/app/strictmode/WhitelistEngine.kt`
around lines 256 - 260, The current matchAdjacentFrames invocation that looks
for classAndMethod("java.io.File","exists") followed by the three MIUI frames
should be narrowed to require the downstream NotificationManager call path as
well; update the matcher that builds this rule (the matchAdjacentFrames call) to
also require, in order, a
classAndMethod("android.app.NotificationManager","notify") or
classAndMethod("android.app.NotificationManager","notifyAsUser") frame after (or
before, as appropriate to call order) the MIUI EpFrameworkFactory frames so the
rule only triggers when the MIUI chain is part of a notification delivery path.
Comment on lines
+173
to
+183
| viewLifecycleOwner.lifecycleScope.launch { | ||
| withContext(Dispatchers.IO) { | ||
| template.widgets.forEach { widget -> | ||
| if (widget is ParameterWidget<*>) { | ||
| widget.parameter.beforeCreateView() | ||
| } | ||
| } | ||
| } | ||
| _binding ?: return@launch | ||
| binding.widgets.adapter = TemplateWidgetsListAdapter(template.widgets) | ||
| } |
Contributor
There was a problem hiding this comment.
Prevent stale adapter assignment from overlapping bind jobs
Line 173 launches a new coroutine per template update, but previous jobs are not canceled. A slower old job can complete later and assign widgets for the wrong template.
Suggested fix
+import kotlinx.coroutines.Job
...
class TemplateDetailsFragment : ... {
+ private var bindTemplateJob: Job? = null
...
private fun bindWithTemplate(template: Template<*>?) {
template ?: return
binding.title.text = template.templateNameStr
- viewLifecycleOwner.lifecycleScope.launch {
+ bindTemplateJob?.cancel()
+ bindTemplateJob = viewLifecycleOwner.lifecycleScope.launch {
withContext(Dispatchers.IO) {
template.widgets.forEach { widget ->
if (widget is ParameterWidget<*>) {
widget.parameter.beforeCreateView()
}
}
}
_binding ?: return@launch
binding.widgets.adapter = TemplateWidgetsListAdapter(template.widgets)
}
}
}🤖 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/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`
around lines 173 - 183, The coroutine launched in
viewLifecycleOwner.lifecycleScope.launch can overlap older runs and assign the
wrong adapter; add a Job field (e.g., widgetsBindJob) on the fragment, cancel
widgetsBindJob?.cancel() before starting a new
viewLifecycleOwner.lifecycleScope.launch, store the returned Job in
widgetsBindJob, do the Dispatchers.IO work (template.widgets.forEach /
ParameterWidget.beforeCreateView) inside that coroutine, and only after the job
is the active one assign binding.widgets.adapter =
TemplateWidgetsListAdapter(template.widgets) (also keep the existing _binding
null-check) so stale/cancelled jobs cannot overwrite the adapter.
Comment on lines
+125
to
+166
| viewLifecycleOwner.lifecycleScope.launch { | ||
| val (templates, warnings) = withContext(Dispatchers.IO) { | ||
| val provider = ITemplateProvider.getInstance(reload = true) | ||
| // Pre-warm the widget view provider so per-bind getInstance() in | ||
| // TemplateWidgetsListAdapter doesn't trigger a disk read on the UI thread. | ||
| ITemplateWidgetViewProvider.getInstance() | ||
| val templates = provider.getTemplates().filterIsInstance<ProjectTemplate>() | ||
| val warnings = (provider as? TemplateProviderImpl)?.warnings.orEmpty() | ||
| templates to warnings | ||
| } | ||
|
|
||
| if (warnings.isNotEmpty()) { | ||
| requireActivity().flashError( | ||
| warnings.joinToString(System.lineSeparator()) { w -> | ||
| requireContext().getString(w.resId, *w.args.toTypedArray()) | ||
| } | ||
| ) | ||
| _binding ?: return@launch | ||
|
|
||
| adapter = | ||
| TemplateListAdapter( | ||
| templates = templates, | ||
| onClick = { template, _ -> | ||
| viewModel.template.value = template | ||
| viewModel.setScreen(MainViewModel.SCREEN_TEMPLATE_DETAILS) | ||
| }, | ||
| onLongClick = { template, itemView -> | ||
| template.tooltipTag?.let { tag -> | ||
| TooltipManager.showIdeCategoryTooltip( | ||
| context = requireContext(), | ||
| anchorView = itemView, | ||
| tag = tag | ||
| ) | ||
| } | ||
| }, | ||
| ) | ||
| binding.list.adapter = adapter | ||
| updateSpanCount() | ||
|
|
||
| if (warnings.isNotEmpty()) { | ||
| requireActivity().flashError( | ||
| warnings.joinToString(System.lineSeparator()) { w -> | ||
| requireContext().getString(w.resId, *w.args.toTypedArray()) | ||
| } | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Cancel previous reload coroutine to avoid out-of-order UI updates
Line 125 starts a new reload every time reloadTemplates() is called, but prior jobs remain active. Older jobs can finish after newer ones and overwrite the list/warnings with stale data.
Suggested fix
+import kotlinx.coroutines.Job
...
class TemplateListFragment : ... {
+ private var reloadJob: Job? = null
...
private fun reloadTemplates() {
_binding ?: return
log.debug("Reloading templates...")
- viewLifecycleOwner.lifecycleScope.launch {
+ reloadJob?.cancel()
+ reloadJob = viewLifecycleOwner.lifecycleScope.launch {
val (templates, warnings) = withContext(Dispatchers.IO) {
...
}
_binding ?: return@launch
...
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| viewLifecycleOwner.lifecycleScope.launch { | |
| val (templates, warnings) = withContext(Dispatchers.IO) { | |
| val provider = ITemplateProvider.getInstance(reload = true) | |
| // Pre-warm the widget view provider so per-bind getInstance() in | |
| // TemplateWidgetsListAdapter doesn't trigger a disk read on the UI thread. | |
| ITemplateWidgetViewProvider.getInstance() | |
| val templates = provider.getTemplates().filterIsInstance<ProjectTemplate>() | |
| val warnings = (provider as? TemplateProviderImpl)?.warnings.orEmpty() | |
| templates to warnings | |
| } | |
| if (warnings.isNotEmpty()) { | |
| requireActivity().flashError( | |
| warnings.joinToString(System.lineSeparator()) { w -> | |
| requireContext().getString(w.resId, *w.args.toTypedArray()) | |
| } | |
| ) | |
| _binding ?: return@launch | |
| adapter = | |
| TemplateListAdapter( | |
| templates = templates, | |
| onClick = { template, _ -> | |
| viewModel.template.value = template | |
| viewModel.setScreen(MainViewModel.SCREEN_TEMPLATE_DETAILS) | |
| }, | |
| onLongClick = { template, itemView -> | |
| template.tooltipTag?.let { tag -> | |
| TooltipManager.showIdeCategoryTooltip( | |
| context = requireContext(), | |
| anchorView = itemView, | |
| tag = tag | |
| ) | |
| } | |
| }, | |
| ) | |
| binding.list.adapter = adapter | |
| updateSpanCount() | |
| if (warnings.isNotEmpty()) { | |
| requireActivity().flashError( | |
| warnings.joinToString(System.lineSeparator()) { w -> | |
| requireContext().getString(w.resId, *w.args.toTypedArray()) | |
| } | |
| ) | |
| } | |
| } | |
| } | |
| } | |
| reloadJob?.cancel() | |
| reloadJob = viewLifecycleOwner.lifecycleScope.launch { | |
| val (templates, warnings) = withContext(Dispatchers.IO) { | |
| val provider = ITemplateProvider.getInstance(reload = true) | |
| // Pre-warm the widget view provider so per-bind getInstance() in | |
| // TemplateWidgetsListAdapter doesn't trigger a disk read on the UI thread. | |
| ITemplateWidgetViewProvider.getInstance() | |
| val templates = provider.getTemplates().filterIsInstance<ProjectTemplate>() | |
| val warnings = (provider as? TemplateProviderImpl)?.warnings.orEmpty() | |
| templates to warnings | |
| } | |
| _binding ?: return@launch | |
| adapter = | |
| TemplateListAdapter( | |
| templates = templates, | |
| onClick = { template, _ -> | |
| viewModel.template.value = template | |
| viewModel.setScreen(MainViewModel.SCREEN_TEMPLATE_DETAILS) | |
| }, | |
| onLongClick = { template, itemView -> | |
| template.tooltipTag?.let { tag -> | |
| TooltipManager.showIdeCategoryTooltip( | |
| context = requireContext(), | |
| anchorView = itemView, | |
| tag = tag | |
| ) | |
| } | |
| }, | |
| ) | |
| binding.list.adapter = adapter | |
| updateSpanCount() | |
| if (warnings.isNotEmpty()) { | |
| requireActivity().flashError( | |
| warnings.joinToString(System.lineSeparator()) { w -> | |
| requireContext().getString(w.resId, *w.args.toTypedArray()) | |
| } | |
| ) | |
| } | |
| } | |
| } |
🤖 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/com/itsaky/androidide/fragments/TemplateListFragment.kt`
around lines 125 - 166, The reloadTemplates coroutine can run multiple
concurrent jobs and older ones may overwrite newer UI state; add a cancellable
Job property (e.g., private var reloadJob: Job? = null) on TemplateListFragment,
cancel any existing reloadJob before starting a new
viewLifecycleOwner.lifecycleScope.launch in reloadTemplates, then assign the
launched Job to reloadJob so only the latest job updates
adapter/binding/warnings; keep the existing _binding check and UI update logic
unchanged.
Comment on lines
+48
to
+53
| TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG) | ||
| val address = try { | ||
| connector.startListening(args) | ||
| } finally { | ||
| TrafficStats.clearThreadStatsTag() | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android TrafficStats getThreadStatsTag clearThreadStatsTag API
💡 Result:
The Android TrafficStats class provides methods for per-thread network traffic tagging to track socket traffic statistics. getThreadStatsTag: Returns the active tag (int) used for accounting Socket traffic from the current thread. Only one tag per thread is supported. Added in API level 14 [1][2]. clearThreadStatsTag: Clears the active thread stats tag for the current thread. Added in API level 8 [1][2][3]. These work with setThreadStatsTag(int tag) to set the tag (API level 8) [1][2], and tagSocket(Socket) to apply it to sockets. Tags help profile app network usage by category [4][5]. Example usage for HTTP requests [5]: TrafficStats.setThreadStatsTag(0xF00D); try { // Make network request } finally { TrafficStats.clearThreadStatsTag(); } Official documentation confirms these are stable APIs as of Android 15+ (current as of 2026) [1][2].
Citations:
- 1: https://developer.android.com/reference/android/net/TrafficStats.html?is-external=true
- 2: https://developer.android.com/reference/android/net/TrafficStats
- 3: https://learn.microsoft.com/en-us/dotnet/api/android.net.trafficstats.clearthreadstatstag?view=net-android-35.0
- 4: https://source.android.com/docs/core/data/tags-explained
- 5: https://stackoverflow.com/questions/36696447/how-to-get-trafficstats-from-a-specific-thread
🌐 Web query:
TrafficStats getThreadStatsTag() return value no tag -1
💡 Result:
In Android's TrafficStats API, getThreadStatsTag returns the active thread stats tag used for accounting socket traffic from the current thread. The return value of -1 indicates no tag is set for the thread, as this is the default initial value in the ThreadLocal storage (UidTag.tag = -1).[1][2] This differs from TrafficStats.UNSUPPORTED (-1), which is returned by network statistic methods when the device does not support the requested statistic.[3][4][5][6] Official docs describe getThreadStatsTag as returning the "active tag" without specifying -1 explicitly as "no tag," but source code confirms -1 as the unset/default state, and developers use clearThreadStatsTag to explicitly reset to this state.[4][1]
Citations:
- 1: https://android.googlesource.com/platform/prebuilts/fullsdk/sources/+/refs/heads/androidx-media-release/android-34/android/net/TrafficStats.java
- 2: https://android.googlesource.com/platform/frameworks/base/+/bcc2d40a11432736e94c1dd3c64abd1aac476c20/core/java/com/android/server/NetworkManagementSocketTagger.java
- 3: https://android.googlesource.com/platform/frameworks/base/+/705d2ea63a4b3c2345af83eec9dabdeea29cfa9f/core/java/android/net/TrafficStats.java
- 4: https://developer.android.com/reference/android/net/TrafficStats
- 5: https://stackoverflow.com/questions/5485586/android-unsupported-the-return-value-to-indicate-that-the-device-does-not-suppor
- 6: https://emanual.github.io/Android-docs/reference/android/net/TrafficStats.html
Preserve and restore any pre-existing thread stats tag
clearThreadStatsTag() always resets to the unset state (−1), which erases any existing tag on pooled threads. Save the previous tag via getThreadStatsTag() and restore it in the finally block to prevent cross-call traffic attribution drift.
💡 Suggested patch
fun startListening(): String {
- TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG)
+ val previousTag = TrafficStats.getThreadStatsTag()
+ TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG)
val address = try {
connector.startListening(args)
} finally {
- TrafficStats.clearThreadStatsTag()
+ if (previousTag == -1) {
+ TrafficStats.clearThreadStatsTag()
+ } else {
+ TrafficStats.setThreadStatsTag(previousTag)
+ }
}
listenAddress = address
return address
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG) | |
| val address = try { | |
| connector.startListening(args) | |
| } finally { | |
| TrafficStats.clearThreadStatsTag() | |
| } | |
| fun startListening(): String { | |
| val previousTag = TrafficStats.getThreadStatsTag() | |
| TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG) | |
| val address = try { | |
| connector.startListening(args) | |
| } finally { | |
| if (previousTag == -1) { | |
| TrafficStats.clearThreadStatsTag() | |
| } else { | |
| TrafficStats.setThreadStatsTag(previousTag) | |
| } | |
| } | |
| listenAddress = address | |
| return address | |
| } |
🤖 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 `@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kt`
around lines 48 - 53, Preserve and restore any existing thread stats tag around
the JDWP listener setup: before calling
TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG) read and save the
current tag with TrafficStats.getThreadStatsTag(), then call
TrafficStats.setThreadStatsTag(...), run connector.startListening(args) and in
the finally block restore the saved tag by calling
TrafficStats.setThreadStatsTag(savedTag) instead of
TrafficStats.clearThreadStatsTag(); reference the
TrafficStats.getThreadStatsTag, TrafficStats.setThreadStatsTag,
TrafficStats.clearThreadStatsTag calls and the connector.startListening(args)
invocation in ListenerState.kt to implement this change.
Comment on lines
+189
to
203
| private var beforeCreateViewInvoked = false | ||
|
|
||
| /** | ||
| * Called before the layout for this widget is created. | ||
| * Called before the layout for this widget is created. The action registered via | ||
| * [doBeforeCreateView] is invoked at most once per parameter instance — callers | ||
| * may pre-invoke this off the UI thread (e.g. before binding a RecyclerView) so | ||
| * that the bind-time call is a no-op and avoids triggering disk reads on the | ||
| * main thread. | ||
| */ | ||
| open fun beforeCreateView() { | ||
| if (beforeCreateViewInvoked) { | ||
| return | ||
| } | ||
| beforeCreateViewInvoked = true | ||
| this.actionBeforeCreateView?.invoke(this) |
Contributor
There was a problem hiding this comment.
Guard one-shot beforeCreateView() with synchronization
Line 199-Line 203 performs a non-atomic check/set on beforeCreateViewInvoked. Concurrent callers can still execute the action twice, violating the one-shot guarantee.
Suggested fix
- private var beforeCreateViewInvoked = false
+ private var beforeCreateViewInvoked = false
open fun beforeCreateView() {
- if (beforeCreateViewInvoked) {
- return
- }
- beforeCreateViewInvoked = true
- this.actionBeforeCreateView?.invoke(this)
+ val action = lock.withLock {
+ if (beforeCreateViewInvoked) return
+ beforeCreateViewInvoked = true
+ actionBeforeCreateView
+ }
+ action?.invoke(this)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private var beforeCreateViewInvoked = false | |
| /** | |
| * Called before the layout for this widget is created. | |
| * Called before the layout for this widget is created. The action registered via | |
| * [doBeforeCreateView] is invoked at most once per parameter instance — callers | |
| * may pre-invoke this off the UI thread (e.g. before binding a RecyclerView) so | |
| * that the bind-time call is a no-op and avoids triggering disk reads on the | |
| * main thread. | |
| */ | |
| open fun beforeCreateView() { | |
| if (beforeCreateViewInvoked) { | |
| return | |
| } | |
| beforeCreateViewInvoked = true | |
| this.actionBeforeCreateView?.invoke(this) | |
| private var beforeCreateViewInvoked = false | |
| /** | |
| * Called before the layout for this widget is created. The action registered via | |
| * [doBeforeCreateView] is invoked at most once per parameter instance — callers | |
| * may pre-invoke this off the UI thread (e.g. before binding a RecyclerView) so | |
| * that the bind-time call is a no-op and avoids triggering disk reads on the | |
| * main thread. | |
| */ | |
| open fun beforeCreateView() { | |
| val action = lock.withLock { | |
| if (beforeCreateViewInvoked) return | |
| beforeCreateViewInvoked = true | |
| actionBeforeCreateView | |
| } | |
| action?.invoke(this) | |
| } |
🤖 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 `@templates-api/src/main/java/com/itsaky/androidide/templates/parameters.kt`
around lines 189 - 203, The one-shot guard in beforeCreateView currently does a
non-atomic check/set on beforeCreateViewInvoked allowing races; make the
check-and-set atomic (e.g., replace the boolean with an AtomicBoolean and use
compareAndSet, or protect the check/set and invocation with a synchronized
block) so that beforeCreateViewInvoked is set and actionBeforeCreateView is
invoked exactly once across threads; update the beforeCreateView method to
atomically test-and-set before invoking actionBeforeCreateView(this).
Comment on lines
+179
to
+190
| root.isErrorEnabled = err != null | ||
| if (err != null) { | ||
| root.error = err | ||
| } | ||
| } ?: run { | ||
| // Fall back to synchronous validation when no lifecycle owner is attached | ||
| // (e.g., previews/tests). Production attaches a fragment lifecycle. | ||
| val err = ConstraintVerifier.verify(value, constraints = param.constraints) | ||
| root.isErrorEnabled = err != null | ||
| if (err != null) { | ||
| root.error = err | ||
| } |
Contributor
There was a problem hiding this comment.
Clear error text when validation passes
At Line 179-Line 190, isErrorEnabled is toggled, but root.error is only set on failure. Clear it on success to avoid stale error state.
Suggested fix
- root.isErrorEnabled = err != null
- if (err != null) {
- root.error = err
- }
+ root.isErrorEnabled = err != null
+ root.error = err🤖 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
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt`
around lines 179 - 190, The validation branch toggles root.isErrorEnabled and
only sets root.error when err != null, leaving stale error text when validation
passes; in both the lifecycle-owner branch and the fallback
(ConstraintVerifier.verify) branch inside TemplateWidgetViewProviderImpl
(references: root, root.isErrorEnabled, root.error,
ConstraintVerifier.verify(value, constraints = param.constraints), value,
param.constraints) ensure that when err == null you explicitly clear the error
by setting root.error = null and set root.isErrorEnabled = false so no stale
error text remains after successful validation.
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.
Address 33 still-reproducible violations from the create-new-project log (the other 16 from legacy noActivity/emptyActivity/basicActivity templates were already removed by ADFA-3381).
Off-thread the heavy work:
Defer initialization:
Tag the JDWP listener socket:
Whitelist vendor-framework call (not app-owned):