From c24adb29503ec78c6d076801e1d82d16140e3413 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Fri, 19 Apr 2013 23:00:00 -0400 Subject: [PATCH] Rework DefaultNotificationService registration. - Instead of implicitly registering and unregistering a service instance on construction/destruction, DefaultNotificationService now exports explicit Register()/Unregister() calls, which subclasses are expected to call when they're ready. - Adjust all implementing subclasses. Resolves an issue with deadlocks when booting a DEBUG=1 build. --- headers/private/kernel/Notifications.h | 3 +++ .../network/notifications/notifications.cpp | 14 ++++++++++++-- src/system/kernel/Notifications.cpp | 16 ++++++++++++++-- .../kernel/device_manager/IOSchedulerRoster.cpp | 2 ++ .../disk_device_manager/KDiskDeviceManager.cpp | 4 ++++ src/system/kernel/image.cpp | 2 ++ src/system/kernel/port.cpp | 1 + src/system/kernel/team.cpp | 2 ++ src/system/kernel/thread.cpp | 2 ++ 9 files changed, 42 insertions(+), 4 deletions(-) diff --git a/headers/private/kernel/Notifications.h b/headers/private/kernel/Notifications.h index 78497afc90..728bbfa35b 100644 --- a/headers/private/kernel/Notifications.h +++ b/headers/private/kernel/Notifications.h @@ -152,6 +152,9 @@ public: virtual const char* Name() { return fName; } + status_t Register(); + void Unregister(); + protected: virtual status_t ToEventMask(const KMessage& eventSpecifier, uint32& eventMask); diff --git a/src/add-ons/kernel/network/notifications/notifications.cpp b/src/add-ons/kernel/network/notifications/notifications.cpp index 6d01979e65..839edab235 100644 --- a/src/add-ons/kernel/network/notifications/notifications.cpp +++ b/src/add-ons/kernel/network/notifications/notifications.cpp @@ -130,22 +130,32 @@ notifications_std_ops(int32 op, ...) { switch (op) { case B_MODULE_INIT: + { TRACE("init\n"); new(&sNotificationService) NetNotificationService(); + status_t result = sNotificationService.Register(); + if (result != B_OK) + return result; register_generic_syscall(NET_NOTIFICATIONS_SYSCALLS, net_notifications_control, 1, 0); return B_OK; - + } case B_MODULE_UNINIT: TRACE("uninit\n"); unregister_generic_syscall(NET_NOTIFICATIONS_SYSCALLS, 1); + // TODO: due to the way the locking in the notification + // manager works, there's a potential race condition here + // where someone attempts to add a listener right as + // we're uninitializing. Needs to be looked at/resolved. + sNotificationService.Unregister(); + // we need to release the reference that was acquired // on our behalf by the NotificationManager. -// sNotificationService.ReleaseReference(); + sNotificationService.ReleaseReference(); sNotificationService.~NetNotificationService(); return B_OK; diff --git a/src/system/kernel/Notifications.cpp b/src/system/kernel/Notifications.cpp index 70ca761c61..0673cbaf84 100644 --- a/src/system/kernel/Notifications.cpp +++ b/src/system/kernel/Notifications.cpp @@ -156,13 +156,11 @@ DefaultNotificationService::DefaultNotificationService(const char* name) fName(name) { recursive_lock_init(&fLock, name); - NotificationManager::Manager().RegisterService(*this); } DefaultNotificationService::~DefaultNotificationService() { - NotificationManager::Manager().UnregisterService(*this); recursive_lock_destroy(&fLock); } @@ -253,6 +251,20 @@ DefaultNotificationService::RemoveListener(const KMessage* eventSpecifier, } +status_t +DefaultNotificationService::Register() +{ + return NotificationManager::Manager().RegisterService(*this); +} + + +void +DefaultNotificationService::Unregister() +{ + NotificationManager::Manager().UnregisterService(*this); +} + + status_t DefaultNotificationService::ToEventMask(const KMessage& eventSpecifier, uint32& eventMask) diff --git a/src/system/kernel/device_manager/IOSchedulerRoster.cpp b/src/system/kernel/device_manager/IOSchedulerRoster.cpp index bacf3bf6cc..5724aa17b6 100644 --- a/src/system/kernel/device_manager/IOSchedulerRoster.cpp +++ b/src/system/kernel/device_manager/IOSchedulerRoster.cpp @@ -78,10 +78,12 @@ IOSchedulerRoster::IOSchedulerRoster() fNotificationService("I/O") { mutex_init(&fLock, "IOSchedulerRoster"); + fNotificationService.Register(); } IOSchedulerRoster::~IOSchedulerRoster() { mutex_destroy(&fLock); + fNotificationService.Unregister(); } diff --git a/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp b/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp index 076a24bda4..8a85992ef2 100644 --- a/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp +++ b/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp @@ -254,6 +254,8 @@ KDiskDeviceManager::KDiskDeviceManager() if (InitCheck() != B_OK) return; + fNotifications->Register(); + RescanDiskSystems(); fMediaChecker = spawn_kernel_thread(_CheckMediaStatusDaemon, @@ -310,6 +312,8 @@ KDiskDeviceManager::~KDiskDeviceManager() delete diskSystem; } + fNotifications->Unregister(); + // delete the containers delete fPartitions; delete fDevices; diff --git a/src/system/kernel/image.cpp b/src/system/kernel/image.cpp index 1de30f7c0a..e76cc60118 100644 --- a/src/system/kernel/image.cpp +++ b/src/system/kernel/image.cpp @@ -343,6 +343,8 @@ image_init(void) new(&sNotificationService) ImageNotificationService(); + sNotificationService.Register(); + #ifdef ADD_DEBUGGER_COMMANDS add_debugger_command("team_images", &dump_images_list, "Dump all registered images from the current team"); #endif diff --git a/src/system/kernel/port.cpp b/src/system/kernel/port.cpp index 26fb39d1e3..e4ba96c7ae 100644 --- a/src/system/kernel/port.cpp +++ b/src/system/kernel/port.cpp @@ -792,6 +792,7 @@ port_init(kernel_args *args) " - address of the port's read or write condition.\n", 0); new(&sNotificationService) PortNotificationService(); + sNotificationService.Register(); sPortsActive = true; return B_OK; } diff --git a/src/system/kernel/team.cpp b/src/system/kernel/team.cpp index d0d68d8c62..1212b62df5 100644 --- a/src/system/kernel/team.cpp +++ b/src/system/kernel/team.cpp @@ -2771,6 +2771,8 @@ team_init(kernel_args* args) new(&sNotificationService) TeamNotificationService(); + sNotificationService.Register(); + return B_OK; } diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp index 93fcfe5dbb..7ac994cd9e 100644 --- a/src/system/kernel/thread.cpp +++ b/src/system/kernel/thread.cpp @@ -2738,6 +2738,8 @@ thread_init(kernel_args *args) // init the notification service new(&sNotificationService) ThreadNotificationService(); + sNotificationService.Register(); + // start the undertaker thread new(&sUndertakerEntries) DoublyLinkedList(); sUndertakerCondition.Init(&sUndertakerEntries, "undertaker entries");