Skip to content

fix: handle notification closed signal properly#1066

Merged
wjyrich merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-352131
Mar 19, 2026
Merged

fix: handle notification closed signal properly#1066
wjyrich merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-352131

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Mar 19, 2026

Added connection to the NotificationClosed signal in initNotifications function to properly handle when notifications are closed. When a notification is closed, the globalNotifyId should be reset to 0 to prevent stale notification ID references. This ensures the system correctly tracks active notifications and avoids potential issues with notification management.

Influence:

  1. Test notification creation and closure scenarios
  2. Verify globalNotifyId is properly reset when notifications are closed
  3. Check for any race conditions in notification ID management
  4. Ensure notification system stability after multiple open/close cycles

fix: 正确处理通知关闭信号

在 initNotifications 函数中添加了对 NotificationClosed 信号的连接,以正 确处理通知关闭事件。当通知被关闭时,需要将 globalNotifyId 重置为 0,以
防止过时的通知ID引用。这确保系统能正确跟踪活动通知,避免通知管理中的潜在
问题。

主要为了,蓝牙模块在连接后,通知发出来后,此时等待通知bubble消失,然后断开蓝牙连接,会导致通知读取不到原来窗口,导致不显示窗口, 目前处理是监听通知横幅关闭,从而重置窗口id,保证断开后能够显示正确一个新窗口。

PMS: BUG-352131

Summary by Sourcery

Bug Fixes:

  • Reset the stored notification ID when the corresponding system notification is closed to avoid stale references and missing Bluetooth notification windows.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 19, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds handling of the NotificationClosed signal in the Bluetooth notification initialization so that the global notification ID is reset when its corresponding notification is closed, preventing stale ID references and improving notification lifecycle correctness.

Sequence diagram for notification closed handling with globalNotifyId reset

sequenceDiagram
    participant User
    participant BluetoothModule
    participant NotificationService

    User->>BluetoothModule: Trigger Bluetooth event
    BluetoothModule->>NotificationService: Create notification
    NotificationService-->>BluetoothModule: Return notification id
    BluetoothModule->>BluetoothModule: Set globalNotifyId = id

    NotificationService-->>BluetoothModule: Signal NotificationClosed(id, reason)
    BluetoothModule->>BluetoothModule: ConnectNotificationClosed handler invoked
    BluetoothModule->>BluetoothModule: Lock globalNotifyMu
    alt Closed notification matches current id
        BluetoothModule->>BluetoothModule: globalNotifyId = 0
    else Different notification id
        BluetoothModule->>BluetoothModule: Leave globalNotifyId unchanged
    end
    BluetoothModule->>BluetoothModule: Unlock globalNotifyMu
Loading

File-Level Changes

Change Details Files
Handle notification closed events to reset the tracked global notification ID safely.
  • Connects to NotificationClosed on the globalNotifications object inside initNotifications
  • On notification close, locks the globalNotifyMu mutex, checks if the closed ID matches globalNotifyId, and resets it to 0 when it matches
  • Logs a warning if the NotificationClosed signal connection fails, similar to existing action-invoked listener error handling
bluetooth1/utils_notify.go

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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In the ConnectNotificationClosed callback, consider using defer globalNotifyMu.Unlock() immediately after globalNotifyMu.Lock() to make the critical section safer and less error-prone if the callback body is modified in the future.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the `ConnectNotificationClosed` callback, consider using `defer globalNotifyMu.Unlock()` immediately after `globalNotifyMu.Lock()` to make the critical section safer and less error-prone if the callback body is modified in the future.

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.

Added connection to the NotificationClosed signal in initNotifications
function to properly handle when notifications are closed. When a
notification is closed, the globalNotifyId should be reset to 0 to
prevent stale notification ID references. This ensures the system
correctly tracks active notifications and avoids potential issues with
notification management.

Influence:
1. Test notification creation and closure scenarios
2. Verify globalNotifyId is properly reset when notifications are closed
3. Check for any race conditions in notification ID management
4. Ensure notification system stability after multiple open/close cycles

fix: 正确处理通知关闭信号

在 initNotifications 函数中添加了对 NotificationClosed 信号的连接,以正
确处理通知关闭事件。当通知被关闭时,需要将 globalNotifyId 重置为 0,以
防止过时的通知ID引用。这确保系统能正确跟踪活动通知,避免通知管理中的潜在
问题。

主要为了,蓝牙模块在连接后,通知发出来后,此时等待通知bubble消失,然后断开蓝牙连接,会导致通知读取不到原来窗口,导致不显示窗口,
目前处理是监听通知横幅关闭,从而重置窗口id,保证断开后能够显示正确一个新窗口。

PMS: BUG-352131
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的 diff 展示了对 bluetooth1/utils_notify.go 文件的修改,主要是更新了版权年份并新增了一个用于监听通知关闭事件的回调处理。以下是对这段代码的审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 版权年份更新:将版权年份从 2018 - 2022 更新至 2018 - 2026 是合理的,符合长期维护项目的惯例。
  • 逻辑正确性
    • 新增的代码逻辑是监听 ConnectNotificationClosed 信号。
    • 当收到信号时,使用互斥锁 globalNotifyMu 保护共享变量 globalNotifyId
    • 检查关闭的通知 ID 是否与当前记录的全局 ID (globalNotifyId) 一致。如果一致,则将其重置为 0。
    • 逻辑流程清晰,判断条件 if globalNotifyId == id 是正确的,防止了误关闭其他通知。

2. 代码质量

  • 错误处理
    • 代码中对 ConnectNotificationClosed 的返回值进行了检查,如果出错则记录 Warning 日志。这是良好的实践。
    • 建议:对于 ConnectNotificationClosed 的连接失败(err != nil),仅仅记录日志可能不够。如果监听失败,后续通知关闭时 globalNotifyId 可能无法正确重置,导致状态不一致。可以考虑根据业务需求决定是否需要重试机制或者更严重的错误上报(例如 Error 级别日志)。
  • 日志格式
    • 日志字符串 "listen notification closed failed,err:%v" 中,逗号后缺少空格(failed, err),建议统一格式为 "listen notification closed failed, err: %v",提高可读性。
  • 变量命名:变量命名清晰,符合 Go 语言规范。

3. 代码性能

  • 锁的粒度
    • 互斥锁 globalNotifyMu 仅用于保护 globalNotifyId 的读写操作,锁的持有时间非常短,仅包含一个简单的 if 判断和赋值。这是非常高效的用法,不会造成性能瓶颈。
  • 回调执行:回调函数会在 DBus 信号触发时执行。由于回调中包含锁操作,如果信号触发极其频繁,可能会对性能产生微小影响,但在通知关闭这种低频事件下,完全可以忽略不计。

4. 代码安全

  • 并发安全
    • 使用了 globalNotifyMu.Lock()globalNotifyMu.Unlock() 来保护对 globalNotifyId 的访问,这是正确的并发控制手段,防止了数据竞争。
    • 潜在风险:虽然目前的锁使用是正确的,但必须确保在代码库的其他所有访问 globalNotifyId 的地方也都使用了同一个互斥锁 globalNotifyMu。如果存在其他地方直接读写该变量而没有加锁,仍然会引发并发安全问题。

改进建议总结

  1. 日志格式微调:建议修改日志字符串,统一标点符号后的空格。

    // 修改前
    logger.Warningf("listen notification closed failed,err:%v", err)
    // 修改后
    logger.Warningf("listen notification closed failed, err: %v", err)
  2. 错误处理增强:虽然目前是 Warning,但建议评估该监听连接失败对业务的影响。如果 globalNotifyId 状态一致性至关重要,建议提升日志级别或在条件允许时增加重试逻辑。

  3. 代码注释:建议在 ConnectNotificationClosed 的回调处添加简短注释,说明为什么需要重置 ID(例如:确保当前通知关闭后状态同步),方便后续维护者理解业务逻辑。

修改后的代码示例(包含注释和日志格式优化):

	_, err = globalNotifications.ConnectNotificationClosed(func(id uint32, reason uint32) {
		globalNotifyMu.Lock()
		// 如果关闭的是当前记录的通知ID,则重置全局ID,防止状态残留
		if globalNotifyId == id {
			globalNotifyId = 0
		}
		globalNotifyMu.Unlock()
	})
	if err != nil {
		// 增加了空格,优化了可读性
		logger.Warningf("listen notification closed failed, err: %v", err)
	}

总体而言,这段代码的修改是正确且安全的,主要改进点在于细节的打磨和健壮性的考量。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@wjyrich wjyrich merged commit c1f483c into linuxdeepin:master Mar 19, 2026
17 of 18 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