diff --git a/core/timermanager.cpp b/core/timermanager.cpp index 7e41aa7444..b07fa9a3c5 100644 --- a/core/timermanager.cpp +++ b/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; } } diff --git a/core/timermanager.h b/core/timermanager.h index 8b7bc1abf1..e78acbea48 100644 --- a/core/timermanager.h +++ b/core/timermanager.h @@ -95,7 +95,7 @@ class my_priority_queue : public std::priority_queue { 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); diff --git a/cpu/ppc/ppcopcodes.cpp b/cpu/ppc/ppcopcodes.cpp index e78ca47ad7..339ede66af 100644 --- a/cpu/ppc/ppcopcodes.cpp +++ b/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; diff --git a/devices/common/ata/atabasedevice.cpp b/devices/common/ata/atabasedevice.cpp index 5aa649f7e4..dbc9e17a78 100644 --- a/devices/common/ata/atabasedevice.cpp +++ b/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); }); diff --git a/devices/common/ata/idechannel.cpp b/devices/common/ata/idechannel.cpp index 68a0afb59f..746454c10d 100644 --- a/devices/common/ata/idechannel.cpp +++ b/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(); }); } diff --git a/devices/common/dbdma.cpp b/devices/common/dbdma.cpp index c0ac1cac7e..f0f3fdbb45 100644 --- a/devices/common/dbdma.cpp +++ b/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); }); } diff --git a/devices/common/scsi/sc53c94.cpp b/devices/common/scsi/sc53c94.cpp index 461a1fa61d..53c16ba3ee 100644 --- a/devices/common/scsi/sc53c94.cpp +++ b/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,7 +477,6 @@ 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) { @@ -487,7 +484,6 @@ void Sc53C94::seq_defer_state(uint64_t delay_ns) 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(); }); @@ -495,7 +491,6 @@ void Sc53C94::seq_defer_state(uint64_t delay_ns) 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 { diff --git a/devices/common/scsi/scsibusctrl.cpp b/devices/common/scsi/scsibusctrl.cpp index b0738d0e4c..21ec97c79a 100644 --- a/devices/common/scsi/scsibusctrl.cpp +++ b/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 { diff --git a/devices/common/scsi/scsihd.cpp b/devices/common/scsi/scsihd.cpp index 296c99d623..f5f637313a 100644 --- a/devices/common/scsi/scsihd.cpp +++ b/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); }); } diff --git a/devices/common/scsi/scsiphysdevice.cpp b/devices/common/scsi/scsiphysdevice.cpp index 9a7bab860c..8b7b148125 100644 --- a/devices/common/scsi/scsiphysdevice.cpp +++ b/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; diff --git a/devices/common/viacuda.cpp b/devices/common/viacuda.cpp index 9fef372731..71cb5a9f81 100644 --- a/devices/common/viacuda.cpp +++ b/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; }); } diff --git a/devices/floppy/swim3.cpp b/devices/floppy/swim3.cpp index ceb2c0aae7..e3e88f7e97 100644 --- a/devices/floppy/swim3.cpp +++ b/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(); diff --git a/devices/ioctrl/amic.cpp b/devices/ioctrl/amic.cpp index 09ec73a2b4..3a310081cf 100644 --- a/devices/ioctrl/amic.cpp +++ b/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); }); } diff --git a/devices/sound/awacs.cpp b/devices/sound/awacs.cpp index be05c74576..34a8129c9b 100644 --- a/devices/sound/awacs.cpp +++ b/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()); } diff --git a/devices/sound/burgundy.cpp b/devices/sound/burgundy.cpp index 583e0eac22..00bdf5535a 100644 --- a/devices/sound/burgundy.cpp +++ b/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; }); diff --git a/devices/sound/soundserver_cubeb.cpp b/devices/sound/soundserver_cubeb.cpp index 6c8f2064fd..bcc8d32db2 100644 --- a/devices/sound/soundserver_cubeb.cpp +++ b/devices/sound/soundserver_cubeb.cpp @@ -227,9 +227,13 @@ int SoundServer::open_out_stream(uint32_t sample_rate, DmaOutChannel *dma_ch) int SoundServer::start_out_stream() { if (is_deterministic) { - LOG_F(9, "Starting sound output deterministic polling."); - TimerManager::get_instance()->add_cyclic_timer( - impl->deterministic_poll_timer, MSECS_TO_NSECS(10), impl->deterministic_poll_cb); + // DMA can restart after a STOP command without closing the output + // stream; the timer may already be running. + if (!impl->deterministic_poll_timer.active) { + LOG_F(9, "Starting sound output deterministic polling."); + TimerManager::get_instance()->add_cyclic_timer( + impl->deterministic_poll_timer, MSECS_TO_NSECS(10), impl->deterministic_poll_cb); + } return 0; } int res = cubeb_stream_start(impl->out_stream); @@ -250,7 +254,6 @@ void SoundServer::close_out_stream() { if (impl->deterministic_poll_timer.active) { TimerManager::get_instance()->cancel_timer(impl->deterministic_poll_timer); - impl->deterministic_poll_timer.active = 0; } if (!impl->out_stream) { impl->status = SND_STREAM_CLOSED; diff --git a/devices/video/appleramdac.cpp b/devices/video/appleramdac.cpp index 4d4a39d2da..24adce6b7f 100644 --- a/devices/video/appleramdac.cpp +++ b/devices/video/appleramdac.cpp @@ -92,7 +92,6 @@ void AppleRamdac::iodev_write(uint32_t address, uint16_t value) { #ifdef CURSOR_LO_DELAY // HACK: prevents artifacts in some cases, disabled by default if (this->cursor_timer.active) { TimerManager::get_instance()->cancel_timer(this->cursor_timer); - cursor_timer.active = 0; } this->cursor_xpos = (value << 8) | this->cursor_pos_lo; #else @@ -105,13 +104,11 @@ void AppleRamdac::iodev_write(uint32_t address, uint16_t value) { if (this->cursor_timer.active) { TimerManager::get_instance()->cancel_timer(this->cursor_timer); this->cursor_xpos = (this->cursor_xpos & 0xff00) | (this->cursor_pos_lo & 0x00ff); - cursor_timer.active = 0; } this->cursor_pos_lo = value; TimerManager::get_instance()->add_oneshot_timer(this->cursor_timer, NS_PER_SEC / 60, [this](uint64_t, uint64_t) { this->cursor_xpos = (this->cursor_xpos & 0xff00) | (this->cursor_pos_lo & 0x00ff); - cursor_timer.active = 0; this->cursor_update_cb(); } ); diff --git a/devices/video/valkyrie.cpp b/devices/video/valkyrie.cpp index 1c815702fb..75afabfb33 100644 --- a/devices/video/valkyrie.cpp +++ b/devices/video/valkyrie.cpp @@ -194,7 +194,6 @@ void ValkyrieVideo::schedule_mode_switch() { TimerManager::get_instance()->add_oneshot_timer(this->mode_timer, MSECS_TO_NSECS(3), [this](uint64_t, uint64_t) { - this->mode_timer.active = 0; this->enable_video_internal(); }); } diff --git a/devices/video/videoctrl.cpp b/devices/video/videoctrl.cpp index 2c779e398f..af8aba9d16 100644 --- a/devices/video/videoctrl.cpp +++ b/devices/video/videoctrl.cpp @@ -130,11 +130,9 @@ void VideoCtrlBase::start_refresh_task() { void VideoCtrlBase::stop_refresh_task() { if (this->refresh_timer.active) { TimerManager::get_instance()->cancel_timer(this->refresh_timer); - this->refresh_timer.active = 0; } if (this->vbl_end_timer.active) { TimerManager::get_instance()->cancel_timer(this->vbl_end_timer); - this->vbl_end_timer.active = 0; } }