Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ class WhitelistRulesTest {
)
}

@Test
fun allow_DiskRead_on_MiuiEpFrameworkFactoryIsEnterpriseJarExists() {
assertAllowed<DiskReadViolation>(
// @formatter:off
stackTraceElement("java.io.File", "exists"),
stackTraceElement("miui.enterprise.EpFrameworkFactory", "isEnterpriseJarExists"),
stackTraceElement("miui.enterprise.EpFrameworkFactory", "get"),
stackTraceElement("miui.enterprise.ApplicationHelperStub\$SingletonHolder", "<clinit>"),
stackTraceElement("miui.enterprise.ApplicationHelperStub", "getInstance"),
stackTraceElement("android.app.NotificationManager", "notifyAsUser"),
// @formatter:on
)
}

@Test
fun allow_DiskRead_on_MtkBoostFwkIsGameApp() {
assertAllowed<DiskReadViolation>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import com.itsaky.androidide.templates.Widget
class TemplateWidgetsListAdapter(
private val widgets: List<Widget<*>>,
) : RecyclerView.Adapter<WidgetViewHolder>() {
private val viewProvider = ITemplateWidgetViewProvider.getInstance()

class WidgetViewHolder(
internal val binding: LayoutTemplateWidgetlistItemBinding,
) : RecyclerView.ViewHolder(binding.root)
Expand All @@ -58,7 +60,6 @@ class TemplateWidgetsListAdapter(
position: Int,
) {
holder.binding.apply {
val viewProvider = ITemplateWidgetViewProvider.getInstance()
val widget = widgets[position]
val view = viewProvider.createView(root.context, widget)
viewProvider.applyCallTooltip { tooltipTag ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,23 @@ object WhitelistEngine {
)
}

rule {
ofType<DiskReadViolation>()
allow(
"""
On MIUI devices, NotificationManager.notify lazily initializes the
EpFrameworkFactory enterprise hook, which checks for an enterprise JAR via
File.exists. This is vendor framework code outside our control, so we allow it.
""".trimIndent(),
)

matchAdjacentFrames(
classAndMethod("java.io.File", "exists"),
classAndMethod("miui.enterprise.EpFrameworkFactory", "isEnterpriseJarExists"),
classAndMethod("miui.enterprise.EpFrameworkFactory", "get"),
)
Comment on lines +256 to +260
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

}

rule {
ofType<DiskReadViolation>()
allow(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.itsaky.androidide.fragments

import android.os.Bundle
import android.view.View
import androidx.lifecycle.lifecycleScope
import org.koin.androidx.viewmodel.ext.android.activityViewModel
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.transition.TransitionManager
Expand All @@ -33,6 +34,7 @@ import com.itsaky.androidide.idetooltips.TooltipTag.SETUP_OVERVIEW
import com.itsaky.androidide.idetooltips.TooltipTag.SETUP_PREVIOUS
import com.itsaky.androidide.roomData.recentproject.RecentProject
import com.itsaky.androidide.tasks.executeAsyncProvideError
import com.itsaky.androidide.templates.ParameterWidget
import com.itsaky.androidide.templates.ProjectTemplateRecipeResult
import com.itsaky.androidide.templates.StringParameter
import com.itsaky.androidide.templates.Template
Expand All @@ -41,6 +43,9 @@ import com.itsaky.androidide.utils.TemplateRecipeExecutor
import com.itsaky.androidide.utils.flashError
import com.itsaky.androidide.utils.flashSuccess
import com.itsaky.androidide.viewmodel.MainViewModel
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext

/**
* A fragment which shows a wizard-like interface for creating templates.
Expand Down Expand Up @@ -160,7 +165,21 @@ class TemplateDetailsFragment :
private fun bindWithTemplate(template: Template<*>?) {
template ?: return

binding.widgets.adapter = TemplateWidgetsListAdapter(template.widgets)
binding.title.text = template.templateNameStr

// Some parameters do disk work in their beforeCreateView hook (e.g. computing a
// non-colliding default project name). Run those hooks on Dispatchers.IO before
// attaching the adapter so that onBindViewHolder skips them (they are one-shot).
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)
}
Comment on lines +173 to +183
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package com.itsaky.androidide.fragments
import android.os.Bundle
import android.view.View
import android.content.res.Configuration
import androidx.lifecycle.lifecycleScope
import org.koin.androidx.viewmodel.ext.android.activityViewModel
import androidx.recyclerview.widget.GridLayoutManager
import com.itsaky.androidide.R
Expand All @@ -28,10 +29,14 @@ import com.itsaky.androidide.databinding.FragmentTemplateListBinding
import com.itsaky.androidide.idetooltips.TooltipManager
import com.itsaky.androidide.idetooltips.TooltipTag.EXIT_TO_MAIN
import com.itsaky.androidide.templates.ITemplateProvider
import com.itsaky.androidide.templates.ITemplateWidgetViewProvider
import com.itsaky.androidide.templates.ProjectTemplate
import com.itsaky.androidide.templates.impl.TemplateProviderImpl
import com.itsaky.androidide.utils.flashError
import com.itsaky.androidide.viewmodel.MainViewModel
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.slf4j.LoggerFactory

/**
Expand Down Expand Up @@ -117,36 +122,46 @@ class TemplateListFragment :

log.debug("Reloading templates...")

val provider = ITemplateProvider.getInstance(reload = true)
val templates = provider.getTemplates().filterIsInstance<ProjectTemplate>()
val warnings = (provider as? TemplateProviderImpl)?.warnings.orEmpty()

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()
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())
}
)
}
}
}
}
Comment on lines +125 to +166
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ internal class JavaDebugAdapter :
IDebugAdapter,
EventConsumer,
AutoCloseable {
private val vmm = Bootstrap.virtualMachineManager()
private val vmm by lazy { Bootstrap.virtualMachineManager() }

private val vms = CopyOnWriteArraySet<VmConnection>()
private val adapterScope = CoroutineScope(Dispatchers.Default)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.itsaky.androidide.lsp.java.debug

import android.net.TrafficStats
import com.itsaky.androidide.lsp.debug.IDebugClient
import com.sun.jdi.VirtualMachine
import com.sun.jdi.connect.Connector
Expand All @@ -14,6 +15,11 @@ internal data class ListenerState(
) {
private val invalidated = AtomicBoolean(false)

private companion object {
// Tag used to identify the JDWP listener socket for traffic-stats accounting.
private const val JDWP_LISTENER_SOCKET_TAG = 0x444A574C // "JDWL"
}

/**
* Whether we're currently listening for incoming connections.
*/
Expand All @@ -39,7 +45,12 @@ internal data class ListenerState(
* @return The address of the listening socket.
*/
fun startListening(): String {
val address = connector.startListening(args)
TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG)
val address = try {
connector.startListening(args)
} finally {
TrafficStats.clearThreadStatsTag()
}
Comment on lines +48 to +53
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 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:


🌐 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:


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.

listenAddress = address
return address
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ abstract class Parameter<T>(

this.actionBeforeCreateView = null
this.actionAfterCreateView = null
this.beforeCreateViewInvoked = false
}

private fun clearObservers() {
Expand Down Expand Up @@ -185,10 +186,20 @@ abstract class Parameter<T>(
this.actionBeforeCreateView = action
}

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)
Comment on lines +189 to 203
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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).

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ import com.itsaky.androidide.templates.impl.databinding.LayoutTextfieldBinding
import com.itsaky.androidide.utils.ServiceLoader
import com.itsaky.androidide.utils.SingleTextWatcher
import androidx.core.graphics.drawable.toDrawable
import androidx.lifecycle.findViewTreeLifecycleOwner
import androidx.lifecycle.lifecycleScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext


/**
Expand All @@ -66,6 +73,8 @@ class TemplateWidgetViewProviderImpl : ITemplateWidgetViewProvider {
@JvmStatic
private var service: ITemplateWidgetViewProvider? = null

private const val VALIDATION_DEBOUNCE_MS = 150L

@JvmStatic
fun getInstance(reload: Boolean = false): ITemplateWidgetViewProvider {
if (reload) {
Expand Down Expand Up @@ -149,20 +158,38 @@ class TemplateWidgetViewProviderImpl : ITemplateWidgetViewProvider {
}
}

var validationJob: Job? = null

param.configureTextField(context, root) { value ->
observer.disableAndRun {
param.setValue(value)
param.resetStartAndEndIcons(root.context, root)
}

val err =
ConstraintVerifier.verify(value, constraints = param.constraints)

root.isErrorEnabled = err != null
if (err != null) {
root.error = err
// The EXISTS / FILE / DIRECTORY constraints stat the filesystem; running them
// synchronously on the UI thread would trigger a StrictMode DiskReadViolation
// on every keystroke. Debounce + dispatch to IO; apply the result on Main.
validationJob?.cancel()
val owner = root.findViewTreeLifecycleOwner()
validationJob = owner?.lifecycleScope?.launch {
delay(VALIDATION_DEBOUNCE_MS)
val err = withContext(Dispatchers.IO) {
ConstraintVerifier.verify(value, constraints = param.constraints)
}
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
}
Comment on lines +179 to +190
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

null
}

}

input.setText(param.value)
Expand Down
Loading