Kernel: Fix GenericInterruptHandler problems with virtual functions

Because registering and unregistering interrupt handlers triggers
calls to virtual functions, we can't do this in the constructor
and destructor.

Fixes #5539

4 files changed, 23 insertions, 4 deletions

diff --git a/Kernel/Time/APICTimer.cpp b/Kernel/Time/APICTimer.cpp
index fb81ba0435..6b68563d5c 100644
--- a/Kernel/Time/APICTimer.cpp
+++ b/Kernel/Time/APICTimer.cpp
@@ -38,12 +38,12 @@ namespace Kernel {
 
 UNMAP_AFTER_INIT APICTimer* APICTimer::initialize(u8 interrupt_number, HardwareTimerBase& calibration_source)
 {
-    auto* timer = new APICTimer(interrupt_number, nullptr);
+    auto timer = adopt(*new APICTimer(interrupt_number, nullptr));
+    timer->register_interrupt_handler();
     if (!timer->calibrate(calibration_source)) {
-        delete timer;
         return nullptr;
     }
-    return timer;
+    return &timer.leak_ref();
 }
 
 UNMAP_AFTER_INIT APICTimer::APICTimer(u8 interrupt_number, Function<void(const RegisterState&)> callback)
diff --git a/Kernel/Time/APICTimer.h b/Kernel/Time/APICTimer.h
index 7e9b24e630..8ed4f147e3 100644
--- a/Kernel/Time/APICTimer.h
+++ b/Kernel/Time/APICTimer.h
@@ -51,6 +51,7 @@ public:
     virtual bool is_capable_of_frequency(size_t frequency) const override;
     virtual size_t calculate_nearest_possible_frequency(size_t frequency) const override;
 
+    void will_be_destroyed() { HardwareTimer<GenericInterruptHandler>::will_be_destroyed(); }
     void enable_local_timer();
     void disable_local_timer();
 
diff --git a/Kernel/Time/HPETComparator.cpp b/Kernel/Time/HPETComparator.cpp
index 78d438bb19..f47f4dca0d 100644
--- a/Kernel/Time/HPETComparator.cpp
+++ b/Kernel/Time/HPETComparator.cpp
@@ -33,7 +33,9 @@ namespace Kernel {
 
 UNMAP_AFTER_INIT NonnullRefPtr<HPETComparator> HPETComparator::create(u8 number, u8 irq, bool periodic_capable)
 {
-    return adopt(*new HPETComparator(number, irq, periodic_capable));
+    auto timer = adopt(*new HPETComparator(number, irq, periodic_capable));
+    timer->register_interrupt_handler();
+    return timer;
 }
 
 UNMAP_AFTER_INIT HPETComparator::HPETComparator(u8 number, u8 irq, bool periodic_capable)
diff --git a/Kernel/Time/HardwareTimer.h b/Kernel/Time/HardwareTimer.h
index c5998e1cb9..5790cf97ab 100644
--- a/Kernel/Time/HardwareTimer.h
+++ b/Kernel/Time/HardwareTimer.h
@@ -49,6 +49,12 @@ class HardwareTimerBase
 public:
     virtual ~HardwareTimerBase() { }
 
+    // We need to create a virtual will_be_destroyed here because we derive
+    // from RefCounted<HardwareTimerBase> here, which means that RefCounted<>
+    // will only call will_be_destroyed if we define it here. The derived
+    // classes then should forward this to e.g. GenericInterruptHandler.
+    virtual void will_be_destroyed() = 0;
+
     virtual const char* model() const = 0;
     virtual HardwareTimerType timer_type() const = 0;
     virtual Function<void(const RegisterState&)> set_callback(Function<void(const RegisterState&)>) = 0;
@@ -73,6 +79,11 @@ class HardwareTimer<IRQHandler>
     : public HardwareTimerBase
     , public IRQHandler {
 public:
+    virtual void will_be_destroyed() override
+    {
+        IRQHandler::will_be_destroyed();
+    }
+
     virtual const char* purpose() const override
     {
         if (TimeManagement::the().is_system_timer(*this))
@@ -115,6 +126,11 @@ class HardwareTimer<GenericInterruptHandler>
     : public HardwareTimerBase
     , public GenericInterruptHandler {
 public:
+    virtual void will_be_destroyed() override
+    {
+        GenericInterruptHandler::will_be_destroyed();
+    }
+
     virtual const char* purpose() const override
     {
         return model();