fix: deduplicate logind shutdown inhibitors during silent updates

[zhaohuiw42]

Use a shared ref-counted inhibitor so overlapping update/check flows only hold one logind inhibit lock at a time, preventing duplicate org.deepin.dde.Lastore1 shutdown blockers.

Bug: https://pms.uniontech.com/bug-view-350063.html

[github-actions]

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴，但您似乎并非合作伙伴的成员或对接人，请联系相关对接人将您添加至组织之中，或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

_{You can retrigger this bot by commenting recheck in this Pull Request}

[deepin-ci-robot]

deepin pr auto review

这份代码 diff 主要对系统的电源抑制锁机制进行了重构，将原有的直接调用 Inhibitor 和 syscall.Close 的方式，改为了基于引用计数的共享锁机制。这是一个很好的改进方向，可以避免文件描述符泄露和重复申请锁的问题。

以下是对代码的详细审查意见，分为语法逻辑、代码质量、代码性能和代码安全四个方面：

1. 语法逻辑

• 共享锁逻辑正确性：
  • sharedInhibitAcquire 和 sharedInhibitRelease 的逻辑看起来是正确的。使用了 sharedInhibitMu 互斥锁保护 sharedInhibitRef 和 sharedInhibitFd，保证了并发安全。
  • 引用计数逻辑符合预期：只有当引用计数从 0 变为 1 时才真正申请锁，只有从 1 变为 0 时才真正释放锁。
• 变量初始化：
  • var sharedInhibitFd dbus.UnixFD = -1 初始化为 -1 是合理的，用于表示无效文件描述符。
  • sharedInhibitRef 初始化为 0 也是合理的。
• 函数变量注入：
  • 使用变量 inhibitorFn 和 closeInhibitFd 是一种很好的做法，便于在单元测试中进行 mock，逻辑上没有问题。

2. 代码质量

• 代码复用与重构：
  • 优点：成功消除了 manager_check.go 和 manager_upgrade.go 中重复的 Inhibitor 调用和 syscall.Close 调用逻辑。将锁的管理集中到了 inhibitor.go 中，符合单一职责原则。
  • 建议：在 manager_check.go 和 manager_upgrade.go 中，inhibit 这个局部函数的逻辑依然非常相似。虽然它们调用的是同一个底层共享函数，但这两个 inhibit 函数本身的代码结构几乎一模一样（只是日志前缀不同）。如果后续还有其他地方需要类似的逻辑，可以考虑进一步抽象，将 inhibit 函数也提升为通用方法，或者接受一个 logPrefix 参数。
• 日志记录：
  • 改进点：原代码在获取锁时会记录 fd，新代码移除了 fd 的日志记录。这是合理的，因为上层调用者不再持有具体的 fd，而是依赖引用计数。日志改为 Prevent shutdown... 更加简洁明了。
  • 细节：在 sharedInhibitAcquire 中，如果 sharedInhibitRef == 0 且获取失败，直接返回了错误。如果 sharedInhibitRef > 0（即共享锁已存在），则只增加计数并返回 sharedInhibitFd。这里没有日志记录"复用已有锁"的信息。建议在 sharedInhibitRef > 0 时也添加一条 Debug 级别的日志，方便排查锁的引用情况，例如：logger.Debugf("Shared inhibit lock reused, ref count: %d", sharedInhibitRef)。
• 错误处理：
  • 在 sharedInhibitAcquire 中，如果 inhibitorFn 调用失败，返回 0 和 err。调用方（如 updateSystemOnChanging）收到错误后会打印日志并返回。这是正确的。
  • 在 sharedInhibitRelease 中，逻辑是先减计数，如果计数不为 0 则直接返回。这意味着如果计数已经为 0（异常情况），函数会直接返回 nil。这虽然不会导致 panic，但掩盖了潜在的逻辑错误（多释放了一次）。建议：如果 sharedInhibitRef == 0，可以考虑记录一条 Warning 日志，提示"尝试释放未持有的锁"，以便发现调用逻辑错误。

3. 代码性能

• 锁竞争：
  • 引入了 sync.Mutex，在 sharedInhibitAcquire 和 sharedInhibitRelease 中会阻塞。考虑到电源抑制锁的申请和释放通常不是高频操作（通常在更新、备份等低频任务中），这里的性能损耗可以忽略不计。
• 系统调用减少：
  • 优点：这是最大的性能提升点。原代码中，如果有多个模块同时需要禁止关机，可能会多次调用 D-Bus 接口申请锁。新代码通过引用计数，保证了整个进程在全局范围内只持有一个实际的 inhibit lock fd，大大减少了与 systemd/login1 的 D-Bus 交互次数和文件描述符的占用。

4. 代码安全

• 资源泄露风险：

  • 改进：原代码中，如果在 Inhibitor 调用成功后，后续逻辑发生 panic，可能会导致 inhibitFd 没有被关闭，从而造成资源泄露（虽然进程退出时系统会回收，但在长时间运行的进程中是不好的）。
  • 现状：新代码中，sharedInhibitFd 是全局变量。如果某个调用者 Acquire 成功后 panic，没有调用 Release，引用计数将永远不会归零，锁永远不会被释放。这会导致系统一直无法关机或休眠。
  • 建议：虽然很难完全防止 panic 导致的资源泄露，但在关键路径（如 updateSystemOnChanging）中，建议使用 defer 来确保锁的释放。目前的代码是在 else if !onChanging 分支中释放，这依赖于逻辑的正确执行。如果逻辑复杂，容易遗漏释放路径。
  • 示例改进思路（伪代码）：

    func (m *Manager) updateSystemOnChanging(onChanging bool, caller methodCaller) {
        // ... 前置逻辑 ...
        if onChanging {
            // ...
            fd, err := sharedInhibitAcquire(why)
            if err != nil {
                // ...
                return
            }
            m.inhibitFd = fd // 这里 m.inhibitFd 似乎只是个标记，实际锁是全局的
        } else {
            // ...
            sharedInhibitRelease()
        }
    }

    目前的实现中，m.inhibitFd 字段在 Manager 结构体中似乎失去了原有的作用（因为不再直接用它来 close，而是用引用计数）。如果 m.inhibitFd 仅用于标记状态，建议改用布尔值或直接移除，以避免混淆。

• 并发安全：

  • 全局变量 sharedInhibitMu 的使用保证了并发安全，没有问题。

总结与建议

这份代码改动质量很高，主要解决了资源管理和代码复用的问题。

主要建议：

1. 增加 Debug 日志：在 sharedInhibitAcquire 复用已有锁时增加日志，便于调试引用计数。
2. 异常检测：在 sharedInhibitRelease 中检测引用计数下溢（即 sharedInhibitRef == 0 时尝试释放）并打印 Warning。
3. 考虑 defer 释放：在调用 sharedInhibitAcquire 的复杂逻辑中，确保有机制（如 defer）保证锁一定会被释放，防止 panic 导致死锁。
4. 清理冗余字段：检查 Manager 结构体中的 inhibitFd 字段，确认其在新的共享锁机制下是否还需要保留，如果仅作标记，建议优化其类型或注释说明其作用。

代码审查结论：通过。逻辑清晰，改进有效，安全性在可控范围内。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: guonafu, noodle424, zhaohuiw42

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

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

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