Avoid enqueueing the deterministic sound poll timer twice

core/timermanager.cpp

@@ -32,14 +32,15 @@ TimerManager* TimerManager::timer_manager;
 
 void TimerManager::add_absolute_timer(TimerInfo &ti, uint64_t timeout_ns, uint64_t interval, timer_cb cb)
 {
+    if (ti.active)
+        LOG_F(ERROR, "Timer %p is already active; cancel it before rescheduling", (void*)&ti);
+
     ti.timeout_ns  = timeout_ns;
     ti.interval_ns = interval;
     ti.active      = true;
     ti.cb          = cb;
 
     // add new timer to the timer queue
-    //if (this->timer_queue.remove_by_id(&ti))
-    //  LOG_F(ERROR, "timer was in queue!");
     this->timer_queue.push(&ti);
 
     // notify listeners about changes in the timer queue
@@ -71,6 +72,7 @@ void TimerManager::add_cyclic_timer(TimerInfo &ti, uint64_t interval, timer_cb c
 void TimerManager::cancel_timer(TimerInfo &ti)
 {
     this->timer_queue.remove_by_id(&ti);
+    ti.active = false;
     if (!this->cb_active) {
         this->notify_timer_changes();
     }
@@ -103,6 +105,9 @@ uint64_t TimerManager::process_timers()
                 timeout_ns_new = time_now + cur_timer->interval_ns;
             cur_timer->timeout_ns = timeout_ns_new;
             this->timer_queue.push(cur_timer);
+        } else {
+            // One-shot callbacks may immediately reuse their TimerInfo.
+            cur_timer->active = false;
         }
 
         this->cb_active = true;
@@ -134,5 +139,6 @@ void TimerManager::cancel_all_timers()
         cur_timer = this->timer_queue.top();
         LOG_F(WARNING, "Canceling timer id:%llu ns:%llu", uint64_t(cur_timer), cur_timer->timeout_ns);
         this->timer_queue.pop();
+        cur_timer->active = false;
     }
 }

core/timermanager.h

@@ -95,7 +95,7 @@ class my_priority_queue : public std::priority_queue<T, Container, Compare> {
 typedef struct TimerInfo {
     uint64_t timeout_ns;  // timer expiry
     uint64_t interval_ns; // 0 for one-shot timers
-    bool     active = false; // set to true when added to the queue
+    bool     active = false; // maintained by TimerManager while queued
     timer_cb cb;          // timer callback
 } TimerInfo;
 
@@ -130,7 +130,9 @@ class TimerManager {
     // return current virtual time in nanoseconds
     uint64_t current_time_ns() const { return get_time_now(); }
 
-    // creating and cancelling timers
+    // Adding requires an inactive timer; cancel before rescheduling.
+    // One-shot timers become inactive before their callback runs.
+    // Cyclic timers remain active until canceled, including during callbacks.
     void add_absolute_timer(TimerInfo &ti, uint64_t timeout_ns, uint64_t interval, timer_cb cb);
     void add_oneshot_timer(TimerInfo &ti, uint64_t timeout, timer_cb cb);
     void add_immediate_timer(TimerInfo &ti, timer_cb cb);

cpu/ppc/ppcopcodes.cpp

@@ -932,7 +932,6 @@ static void trigger_decrementer_exception(uint64_t = 0, uint64_t = 0) {
 }
 
 static void trigger_timed_decrementer_exception(uint64_t, uint64_t) {
-    decrementer_timer.active = 0;
     uint32_t new_val = calc_dec_value();
     if (new_val >= 0 && new_val != uint32_t(-1)) {
         new_val = -1;
@@ -942,7 +941,6 @@ static void trigger_timed_decrementer_exception(uint64_t, uint64_t) {
 }
 
 static void trigger_immediate_decrementer_exception(uint64_t, uint64_t) {
-    decrementer_timer.active = 0;
     update_decrementer(false, ppc_state.spr[SPR::DEC_S], ppc_state.spr[SPR::DEC_S]);
     trigger_decrementer_exception();
 }
@@ -987,7 +985,6 @@ static void update_decrementer(bool update_time_stamp, uint32_t oldval, uint32_t
 }
 
 static void update_thermal(uint64_t, uint64_t) {
-    thermal_timer.active = 0;
 
     uint32_t val = ppc_state.spr[SPR::THRM3];
     auto sampled_interval_timer_value = (val >> 1) & 0x1FFF;

devices/common/ata/atabasedevice.cpp

@@ -80,7 +80,6 @@ uint16_t AtaBaseDevice::read(const uint8_t reg_addr) {
                     if (!this->read_data_timer.active) {
                         TimerManager::get_instance()->add_oneshot_timer(this->read_data_timer,
                             USECS_TO_NSECS(100), [this](uint64_t, uint64_t) {
-                                this->read_data_timer.active = 0;
                                 this->update_intrq(1);
                             }
                         );
@@ -131,7 +130,6 @@ void AtaBaseDevice::write(const uint8_t reg_addr, const uint16_t value) {
                         LOG_F(ERROR, "%s: write_done_timer is already active", this->get_name().c_str());
                     TimerManager::get_instance()->add_oneshot_timer(write_done_timer,
                         USECS_TO_NSECS(100), [this](uint64_t, uint64_t) {
-                            write_done_timer.active = 0;
                             this->r_status &= ~BSY;
                             this->update_intrq(1);
                         }
@@ -144,7 +142,6 @@ void AtaBaseDevice::write(const uint8_t reg_addr, const uint16_t value) {
                         LOG_F(ERROR, "%s: write_more_timer is already active", this->get_name().c_str());
                     TimerManager::get_instance()->add_oneshot_timer(this->write_more_timer,
                         USECS_TO_NSECS(100), [this](uint64_t, uint64_t) {
-                            this->write_more_timer.active = 0;
                             this->signal_data_ready();
                         }
                     );
@@ -232,7 +229,6 @@ int AtaBaseDevice::pull_data(uint8_t *buf, int len) {
         if (this->dma_pull_timer.active)
             LOG_F(ERROR, "%s: dma_pull_timer is already active", this->get_name().c_str());
         TimerManager::get_instance()->add_oneshot_timer(dma_pull_timer, 500, [this](uint64_t, uint64_t) {
-            dma_pull_timer.active = 0;
             this->r_status &= ~(BSY | DRQ);
             this->update_intrq(1);
         });
@@ -262,7 +258,6 @@ int AtaBaseDevice::push_data(uint8_t *buf, int len) {
         if (this->dma_push_timer.active)
             LOG_F(ERROR, "%s: dma_push_timer is already active", this->get_name().c_str());
         TimerManager::get_instance()->add_oneshot_timer(dma_push_timer, 500, [this](uint64_t, uint64_t) {
-            dma_push_timer.active = 0;
             this->r_status &= ~(BSY | DRQ);
             this->update_intrq(1);
         });

devices/common/ata/idechannel.cpp

@@ -95,7 +95,6 @@ void IdeChannel::assert_dmareq(uint64_t delay) {
         LOG_F(WARNING, "%s: dmareq_timer is already active", this->get_name().c_str());
     TimerManager::get_instance()->add_oneshot_timer(this->dmareq_timer, delay, [this](uint64_t, uint64_t) {
         //LOG_F(INFO, "%s: DMAREQ asserted", this->name.c_str());
-        this->dmareq_timer.active = false;
         this->channel_obj->xfer_retry();
     });
 }

devices/common/dbdma.cpp

@@ -293,7 +293,6 @@ void DMAChannel::update_irq(uint8_t cmd_bits) {
                 if (this->int_ctrl) {
                     if (!this->interrupt_timer.active) {
                         TimerManager::get_instance()->add_immediate_timer(this->interrupt_timer, [this](uint64_t, uint64_t) {
-                            this->interrupt_timer.active = 0;
                             this->int_ctrl->ack_dma_int(this->irq_id, 1);
                         });
                     }

devices/common/scsi/sc53c94.cpp

@@ -323,12 +323,10 @@ void Sc53C94::exec_command()
         // release RST line after 25 us
         if (my_timer.active) {
             TimerManager::get_instance()->cancel_timer(this->my_timer);
-            my_timer.active = 0;
         }
         TimerManager::get_instance()->add_oneshot_timer(my_timer,
             USECS_TO_NSECS(25),
             [this](uint64_t, uint64_t) {
-                my_timer.active = 0;
                 this->bus_obj->release_ctrl_line(this->my_bus_id, SCSI_CTRL_RST);
         });
         if (!(config1 & CFG1_DISR)) {
@@ -479,23 +477,20 @@ void Sc53C94::seq_defer_state(uint64_t delay_ns)
 {
     if (this->seq_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->seq_timer);
-        this->seq_timer.active = 0;
     }
 
     if (delay_ns) {
         TimerManager::get_instance()->add_oneshot_timer(this->seq_timer,
             delay_ns,
             [this](uint64_t, uint64_t) {
                 // re-enter the sequencer with the state specified in next_state
-                this->seq_timer.active = 0;
                 this->cur_state = this->next_state;
                 this->sequencer();
         });
     } else {
         TimerManager::get_instance()->add_immediate_timer(this->seq_timer,
             [this](uint64_t, uint64_t) {
                 // re-enter the sequencer with the state specified in next_state
-                this->seq_timer.active = 0;
                 this->cur_state = this->next_state;
                 this->sequencer();
         });
@@ -711,7 +706,6 @@ void Sc53C94::notify(ScsiNotification notif_type, int param)
         if (this->target_id == param) {
             // cancel selection timeout timer
             TimerManager::get_instance()->cancel_timer(this->seq_timer);
-            this->seq_timer.active = 0;
             this->cur_state = SeqState::SEL_END;
             this->sequencer();
         } else {

devices/common/scsi/scsibusctrl.cpp

@@ -33,13 +33,11 @@ using namespace Scsi_Bus_Controller;
 void ScsiBusController::seq_defer_state(uint64_t delay_ns) {
     if (this->seq_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->seq_timer);
-        this->seq_timer.active = 0;
     }
     TimerManager::get_instance()->add_oneshot_timer(this->seq_timer,
         delay_ns,
         [this](uint64_t, uint64_t) {
             // re-enter the sequencer with the state specified in next_state
-            this->seq_timer.active = 0;
             this->cur_state = this->next_state;
             this->sequencer();
     });
@@ -176,7 +174,6 @@ void ScsiBusController::notify(ScsiNotification notif_type, int param) {
         if (this->dst_id == param) {
             // cancel selection timeout timer
             TimerManager::get_instance()->cancel_timer(this->seq_timer);
-            this->seq_timer.active = false;
             this->cur_state = SeqState::SEL_END;
             this->sequencer();
         } else {

devices/common/scsi/scsihd.cpp

@@ -180,7 +180,6 @@ void ScsiHardDisk::format() {
     if (this->phase_switch_timer.active)
         LOG_F(WARNING, "%s: phase_switch_timer is already active", this->get_name().c_str());
     TimerManager::get_instance()->add_oneshot_timer(this->phase_switch_timer, NS_PER_SEC, [this](uint64_t, uint64_t) {
-        this->phase_switch_timer.active = 0;
         this->switch_phase(ScsiPhase::STATUS);
     });
 }

devices/common/scsi/scsiphysdevice.cpp

@@ -42,7 +42,6 @@ void ScsiPhysDevice::notify(ScsiNotification notif_type, int param)
                 TimerManager::get_instance()->add_oneshot_timer(bus_settle_timer,
                     BUS_SETTLE_DELAY,
                     [this](uint64_t, uint64_t) {
-                        this->bus_settle_timer.active = 0;
                         // don't confirm selection if BSY or I/O are asserted
                         if (this->bus_obj->test_ctrl_lines(SCSI_CTRL_BSY | SCSI_CTRL_IO))
                             return;

devices/common/viacuda.cpp

@@ -105,19 +105,15 @@ ViaCuda::~ViaCuda()
 {
     if (this->sr_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->sr_timer);
-        this->sr_timer.active = 0;
     }
     if (this->t1_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->t1_timer);
-        this->t1_timer.active = 0;
     }
     if (this->t2_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->t2_timer);
-        this->t2_timer.active = 0;
     }
     if (this->treq_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->treq_timer);
-        this->treq_timer.active = 0;
     }
 }
 
@@ -216,7 +212,6 @@ void ViaCuda::write(int reg, uint8_t value) {
         // cancel active T1 timer task
         if (this->t1_timer.active) {
             TimerManager::get_instance()->cancel_timer(this->t1_timer);
-            this->t1_timer.active = 0;
         }
         // clear T1 flag in IFR
         this->_via_ifr &= ~VIA_IF_T1;
@@ -243,7 +238,6 @@ void ViaCuda::write(int reg, uint8_t value) {
         // cancel active T2 timer task
         if (this->t2_timer.active) {
             TimerManager::get_instance()->cancel_timer(this->t2_timer);
-            this->t2_timer.active = 0;
         }
         // clear T2 flag in IFR
         this->_via_ifr &= ~VIA_IF_T2;
@@ -257,7 +251,6 @@ void ViaCuda::write(int reg, uint8_t value) {
         TimerManager::get_instance()->add_oneshot_timer(this->t2_timer,
             (this->via_clk_dur * (this->t2_counter + 3) + (uint64_t(1) << 36)) >> 37,
             [this](uint64_t, uint64_t) {
-                this->t2_timer.active = 0;
                 this->assert_t2_int();
             }
         );
@@ -313,7 +306,6 @@ void ViaCuda::activate_t1() {
         [this](uint64_t, uint64_t) {
             // reload the T1 counter from the corresponding latches
             this->t1_counter = (this->via_t1lh << 8) | this->via_t1ll;
-            this->t1_timer.active = 0;
             this->assert_t1_int();
             if (this->via_acr & 0x40)
                 this->activate_t1();
@@ -388,12 +380,10 @@ void ViaCuda::assert_ctrl_line(ViaLine line)
 void ViaCuda::schedule_sr_int(uint64_t timeout_ns) {
     if (this->sr_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->sr_timer);
-        this->sr_timer.active = 0;
     }
     TimerManager::get_instance()->add_oneshot_timer(this->sr_timer,
         timeout_ns,
         [this](uint64_t, uint64_t) {
-            this->sr_timer.active = 0;
             this->assert_sr_int();
         }
     );
@@ -431,7 +421,6 @@ void ViaCuda::write(uint8_t new_state) {
                     [this](uint64_t, uint64_t) {
                         this->via_portb &= ~CUDA_TREQ; // assert TREQ
                         this->treq = 0;
-                        this->treq_timer.active = 0;
                 });
             }
 

devices/floppy/swim3.cpp

@@ -72,15 +72,12 @@ void Swim3Ctrl::reset()
 
     if (this->one_us_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->one_us_timer);
-        this->one_us_timer.active = 0;
     }
     if (this->step_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->step_timer);
-        this->step_timer.active = 0;
     }
     if (this->access_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->access_timer);
-        this->access_timer.active = 0;
     }
 }
 
@@ -292,7 +289,6 @@ void Swim3Ctrl::stop_stepping()
     // cancel stepping task
     if (this->step_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->step_timer);
-        this->step_timer.active = 0;
     }
     this->step_count = 0; // not sure this one is required
 }
@@ -322,7 +318,6 @@ void Swim3Ctrl::start_disk_access()
     TimerManager::get_instance()->add_oneshot_timer(this->access_timer,
         this->int_drive->sync_to_disk(),
         [this](uint64_t, uint64_t) {
-            this->access_timer.active = 0;
             this->cur_state = SWIM3_ADDR_MARK_SEARCH;
             this->disk_access();
         }
@@ -375,7 +370,6 @@ void Swim3Ctrl::disk_access()
     TimerManager::get_instance()->add_oneshot_timer(this->access_timer,
         delay,
         [this](uint64_t, uint64_t) {
-            this->access_timer.active = 0;
             this->disk_access();
         }
     );
@@ -386,7 +380,6 @@ void Swim3Ctrl::stop_disk_access()
     // cancel disk access timer
     if (this->access_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->access_timer);
-        this->access_timer.active = 0;
     }
 }
 
@@ -408,7 +401,6 @@ void Swim3Ctrl::init_timer(const uint8_t start_val)
     TimerManager::get_instance()->add_oneshot_timer(this->one_us_timer,
         uint32_t(this->timer_val) * NS_PER_USEC,
         [this](uint64_t, uint64_t) {
-            this->one_us_timer.active = 0;
             this->timer_val = 0;
             this->int_flags |= INT_TIMER_DONE;
             update_irq();

devices/ioctrl/amic.cpp

@@ -98,7 +98,6 @@ AMIC::~AMIC()
 {
     if (this->pseudo_vbl_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->pseudo_vbl_timer);
-        this->pseudo_vbl_timer.active = 0;
     }
 }
 
@@ -638,7 +637,6 @@ void AmicSndOutDma::update_irq() {
         if (this->interrupt_timer.active)
             LOG_F(ERROR, "AMIC: interrupt_timer is already active");
         TimerManager::get_instance()->add_immediate_timer(interrupt_timer, [this](uint64_t, uint64_t) {
-            interrupt_timer.active = 0;
             this->int_ctrl->ack_dma_int(this->snd_dma_irq_id, this->irq_level);
         });
     }

devices/sound/awacs.cpp

@@ -120,7 +120,6 @@ void AwacsBase::dma_in_data() {
             10000,
             [this](uint64_t, uint64_t) {
                 // re-enter the sequencer with the state specified in next_state
-                this->dma_in_timer.active = 0;
                 this->dma_in_data();
         });
     }
@@ -134,7 +133,6 @@ void AwacsBase::dma_in_start() {
 void AwacsBase::dma_in_stop() {
     if (this->dma_in_timer.active) {
         TimerManager::get_instance()->cancel_timer(this->dma_in_timer);
-        this->dma_in_timer.active = 0;
     }
     LOG_F(ERROR, "%s: dma_in_stop", this->name.c_str());
 }

devices/sound/burgundy.cpp

@@ -118,7 +118,6 @@ void BurgundyCodec::snd_ctrl_write(uint32_t offset, uint32_t value, int size) {
             TimerManager::get_instance()->add_oneshot_timer(this->first_valid_timer,
                 USECS_TO_NSECS(22), // average is approximately 22.6 µs on a real B&W G3
                 [this](uint64_t, uint64_t) {
-                    this->first_valid_timer.active = 0;
                     this->first_valid  = false;
                     this->byte_counter = (this->byte_counter + 1) & 3;
             });