Skip to content

fix(inputdevices): fsync udev rule and rebuild on boot for touchpad - #1225

Merged
fly602 merged 1 commit into
linuxdeepin:release/2500from
fly602:release/2500
Aug 27, 2026
Merged

fix(inputdevices): fsync udev rule and rebuild on boot for touchpad#1225
fly602 merged 1 commit into
linuxdeepin:release/2500from
fly602:release/2500

Conversation

@fly602

@fly602 fly602 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor
  1. Replace os.WriteFile with os.Create + f.Sync() to ensure the udev rule file is flushed to disk before close
  2. On startup, call setTouchpadEnableViaUdev directly instead of setTouchpadEnable to avoid skipped udev rebuild when changed=false due to pre-initialized Enable field
  3. Prevents touchpad disable state loss after forced power-off

Log: fix touchpad disabled state lost after forced power-off because udev rule file write was not fsynced and startup rebuild was skipped by changed=false check

Influence:

  1. Disable touchpad, force power off, reboot and verify it stays disabled
  2. Disable touchpad, normal reboot and verify it stays disabled
  3. Enable touchpad, force power off, reboot and verify it stays enabled

fix(inputdevices): 修复强制关机后触控板禁用状态丢失

  1. 将 os.WriteFile 改为 os.Create + f.Sync() 确保写入 udev 规则文件时强制刷盘,防止断电丢失
  2. 启动恢复时直接调用 setTouchpadEnableViaUdev 重建 udev 规 则,避免 changed=false 导致跳过写入
  3. 修复强制关机后触控板禁用失效的问题

Log: 修复强制关机后触控板禁用状态丢失,原因是 udev 规则文件
写入未调用 fsync 导致断电丢失,且启动恢复逻辑因 changed=false
跳过了 udev 规则重建

Influence:

  1. 禁用触控板后强制关机,重启后确认触控板仍为禁用状态
  2. 禁用触控板后正常重启,确认触控板仍为禁用状态
  3. 启用触控板后强制关机,重启后确认触控板仍为启用状态

PMS: BUG-374789

Summary by Sourcery

Ensure touchpad udev rules are durably persisted and restored so the enabled or disabled state survives power loss and reboot.

Bug Fixes:

  • Persist touchpad enablement state reliably across forced and normal reboots by ensuring udev rule changes are durably written and rebuilt during startup.

Enhancements:

  • Make udev rule updates safer and more robust through atomic creation, permission handling, content-change detection, and parent-directory synchronization.

Build:

  • Update the go-dbus-factory dependency version.

1. Replace os.WriteFile with os.Create + f.Sync() to ensure
   the udev rule file is flushed to disk before close
2. On startup, call setTouchpadEnableViaUdev directly instead
   of setTouchpadEnable to avoid skipped udev rebuild when
   changed=false due to pre-initialized Enable field
3. Prevents touchpad disable state loss after forced power-off

Log: fix touchpad disabled state lost after forced power-off
because udev rule file write was not fsynced and startup rebuild
was skipped by changed=false check

Influence:
1. Disable touchpad, force power off, reboot and verify it
   stays disabled
2. Disable touchpad, normal reboot and verify it stays disabled
3. Enable touchpad, force power off, reboot and verify it stays
   enabled

fix(inputdevices): 修复强制关机后触控板禁用状态丢失

1. 将 os.WriteFile 改为 os.Create + f.Sync() 确保写入 udev
   规则文件时强制刷盘,防止断电丢失
2. 启动恢复时直接调用 setTouchpadEnableViaUdev 重建 udev 规
   则,避免 changed=false 导致跳过写入
3. 修复强制关机后触控板禁用失效的问题

Log: 修复强制关机后触控板禁用状态丢失,原因是 udev 规则文件
写入未调用 fsync 导致断电丢失,且启动恢复逻辑因 changed=false
跳过了 udev 规则重建

Influence:
1. 禁用触控板后强制关机,重启后确认触控板仍为禁用状态
2. 禁用触控板后正常重启,确认触控板仍为禁用状态
3. 启用触控板后强制关机,重启后确认触控板仍为启用状态

PMS: BUG-374789
@sourcery-ai

sourcery-ai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens touchpad udev-rule persistence against forced power loss by syncing file data and creation metadata, and ensures startup always reconstructs the rule from the persisted setting rather than incorrectly skipping the write when the in-memory state already matches.

Sequence diagram for durable touchpad udev rule restoration

sequenceDiagram
    participant Startup
    participant InputDevices
    participant Touchpad
    participant UdevRule
    participant DeviceManager

    Startup->>InputDevices: init()
    InputDevices->>Touchpad: setTouchpadEnableViaUdev(enabled)
    alt enabled
        Touchpad->>UdevRule: remove rule file
        Touchpad->>DeviceManager: refreshTouchpadDevices()
    else disabled
        Touchpad->>UdevRule: writeUdevRuleFile(content)
        UdevRule-->>UdevRule: f.Sync()
        UdevRule-->>UdevRule: sync parent directory
        alt content changed
            Touchpad->>DeviceManager: refreshTouchpadDevices()
        end
    end
Loading

File-Level Changes

Change Details Files
Make udev rule persistence resilient to forced power loss by synchronously writing file contents and, for newly created files, the parent directory entry.
  • Replace whole-file WriteFile with controlled open/create, permission hardening, truncate, write, file Sync, and close handling.
  • Detect whether the rule file was newly created and sync its parent directory; best-effort sync the directory when an unchanged file is reused.
  • Preserve the unchanged-content fast path while enforcing 0644 permissions and avoiding unnecessary device refreshes.
system/inputdevices1/touchpad.go
Rebuild the touchpad udev rule unconditionally during startup restoration instead of routing through state-change detection.
  • Call the direct udev restoration path with the persisted dconfig value.
  • Avoid the pre-initialized Enable field causing changed=false and skipping rule reconstruction after an unclean shutdown.
system/inputdevices1/inputdevices.go
Update the generated D-Bus factory dependency and corresponding checksums.
  • Bump go-dbus-factory to a newer revision.
  • Refresh go.sum entries for the dependency update.
go.mod
go.sum

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 3 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="system/inputdevices1/touchpad.go" line_range="129-130" />
<code_context>
-			logger.Debug("udev rule file already exists with correct content, skip writing")
-			return nil
+		return t.refreshTouchpadDevices()
+	}
+
+	// 禁用:写入 udev 规则文件;若内容未变则无需刷新设备
</code_context>
<issue_to_address>
**issue (bug_risk):** The enabled path removes the udev rule but never synchronizes the parent directory, so a forced power-off after enabling can leave the unlink operation non-persistent and restore the old disabling rule on the next boot.

**Triggers:** When the touchpad is enabled and the machine is powered off before the directory metadata is durably committed.

**Suggested fix:** Call `syncDirBestEffort(filepath.Dir(udevRuleFile))` after a successful `os.Remove`, or synchronously propagate a directory `Sync` error if durability is required.

```suggestion
		if err := os.Remove(udevRuleFile); err != nil {
			if !os.IsNotExist(err) {
				return err
			}
		} else {
			syncDirBestEffort(filepath.Dir(udevRuleFile))
		}
```
</issue_to_address>

### Comment 2
<location path="system/inputdevices1/inputdevices.go" line_range="101" />
<code_context>
+		// 不调用 setTouchpadEnable,因为 newTouchpad 已将 Enable 设为 dconfig 值,
+		// setPropEnable 会判定 changed=false 从而跳过 udev 写入,
+		// 导致强制关机后 udev 规则文件丢失但无法重建。
+		err = m.touchpad.setTouchpadEnableViaUdev(enabled)
 		if err != nil {
 			logger.Warning(err)
</code_context>
<issue_to_address>
**issue (bug_risk):** The startup goroutine uses a snapshot of the dconfig value without synchronizing with `SetTouchpadEnable`; a concurrent user request can be overwritten by the stale startup write, leaving the udev rule and the exposed `Enable` state inconsistent.

**Triggers:** When a touchpad enable/disable request arrives while the asynchronous startup restoration is running.

**Suggested fix:** Serialize startup restoration and `SetTouchpadEnable` with the touchpad mutex, or re-read and validate the current configuration before applying the startup udev state.
</issue_to_address>

### Comment 3
<location path="system/inputdevices1/touchpad.go" line_range="184-188" />
<code_context>
+		f.Close()
+		return false, err
+	}
+	if err := f.Truncate(0); err != nil {
+		f.Close()
+		return false, err
+	}
+	if _, err := f.Write([]byte(udevRuleContent)); err != nil {
+		f.Close()
+		return false, err
</code_context>
<issue_to_address>
**issue (bug_risk):** The replacement path truncates the live udev rule file before writing the new contents, so a process crash, power loss, or concurrent udev read during this window leaves an empty or partial rule file instead of the previous valid rule; the comment claiming an atomic write is false.

**Triggers:** When the machine loses power or another process reads the rule while the file is being rewritten.

**Suggested fix:** Write the contents to a temporary file in the same directory, `Sync` it, then atomically replace the rule with `Rename` and synchronize the parent directory.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread system/inputdevices1/touchpad.go
Comment thread system/inputdevices1/inputdevices.go
Comment thread system/inputdevices1/touchpad.go
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 98 分 (通过阈值: 70分)

Fail


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 代码修复了强制关机后触控板禁用状态丢失的问题,通过 fsync 确保数据落盘并修复启动恢复逻辑。代码质量高,注释详尽,安全处理完善,未发现安全漏洞。存在少量轻微优化建议。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 代码语法正确,编译无问题。逻辑路径清晰:O_EXCL 原子检测新文件 -> Chmod 先于 Truncate 避免数据丢失 -> Write -> Sync -> Close -> 父目录 Sync。错误处理路径均正确关闭文件句柄,无资源泄漏。inputdevices.go 中将 setTouchpadEnable 改为 setTouchpadEnableViaUdev 的逻辑修复正确,解决了 changed=false 导致 udev 规则跳过重建的问题。


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 注释质量优秀,writeUdevRuleFile 函数注释详细说明了 TOCTOU 竞态防护、Chmod 先于 Truncate 的原因、fsync 的必要性、父目录 fsync 的场景等安全考量。函数命名清晰(writeUdevRuleFile、syncDirBestEffort),语义准确。无重复代码,无残留调试代码。go.mod/go.sum 仅更新依赖版本,无质量问题。


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: fsync 引入的 I/O 延迟是必要的,确保断电数据持久化,符合本次修复目的。快速路径(内容已正确)避免不必要的写入和设备刷新,优化合理。此为配置路径非热路径,性能影响可忽略。


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: 安全处理优秀:1) O_EXCL 原子操作避免 TOCTOU 竞态;2) 文件权限 0644 正确设置且在快速路径中兜底收窄,防止外部篡改为 0666 等过宽权限;3) Chmod 先于 Truncate 避免权限修改失败时 O_TRUNC 已清空原内容导致规则丢失;4) fsync 文件数据 + 父目录 fsync 确保断电持久化,解决核心问题;5) 所有错误路径正确关闭文件句柄,无资源泄漏。


💡 改进建议代码示例

// 暂无代码示例

本报告由 AI 代码审查工具自动生成

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fly602
fly602 merged commit 9e30c63 into linuxdeepin:release/2500 Aug 27, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants