Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticks ms #9

Merged
merged 3 commits into from
Oct 13, 2016
Merged

Ticks ms #9

merged 3 commits into from
Oct 13, 2016

Conversation

tannewt
Copy link
Member

@tannewt tannewt commented Oct 13, 2016

No description provided.

Copy link
Member Author

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Tony this looks good overall. Just a couple nitpicky things before submitting it.

systick_ticks_ms += 1;
// Keep the counter within the range of 31 bit uint values since that's the
// max value for micropython 'small' ints.
systick_ticks_ms = systick_ticks_ms > 2147483647L ? 0 : systick_ticks_ms;
Copy link
Member Author

Choose a reason for hiding this comment

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

Can you switch this constant to either a bit shift (1 << 31) or hex? I think it'd be more readable that way.

// Restore ASF-based interrupts.
cpu_irq_restore(irq_flags);
// Restore SysTick interrupt.
SysTick->CTRL |= SysTick_CTRL_TICKINT_Msk;
Copy link
Member Author

Choose a reason for hiding this comment

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

Do you want to swap these two enables so its the reverse order of disable?

@tannewt tannewt merged commit d451f69 into master Oct 13, 2016
@dhalbert dhalbert deleted the ticks_ms branch May 8, 2018 18:38
tannewt pushed a commit that referenced this pull request Jul 7, 2020
jepler added a commit to jepler/circuitpython that referenced this pull request Oct 1, 2020
It was incorrect to NULL out the pointer to our heap allocated buffer in
`reset`, because subsequent to framebuffer_reset, but while
the heap was still active, we could call `get_bufinfo` again,
leading to a fresh allocation on the heap that is about to be destroyed.

Typical stack trace:
```
#1  0x0006c368 in sharpdisplay_framebuffer_get_bufinfo
#2  0x0006ad6e in _refresh_display
#3  0x0006b168 in framebufferio_framebufferdisplay_background
#4  0x00069d22 in displayio_background
adafruit#5  0x00045496 in supervisor_background_tasks
adafruit#6  0x000446e8 in background_callback_run_all
adafruit#7  0x00045546 in supervisor_run_background_tasks_if_tick
adafruit#8  0x0005b042 in common_hal_neopixel_write
adafruit#9  0x00044c4c in clear_temp_status
adafruit#10 0x000497de in spi_flash_flush_keep_cache
adafruit#11 0x00049a66 in supervisor_external_flash_flush
adafruit#12 0x00044b22 in supervisor_flash_flush
adafruit#13 0x0004490e in filesystem_flush
adafruit#14 0x00043e18 in cleanup_after_vm
adafruit#15 0x0004414c in run_repl
adafruit#16 0x000441ce in main
```
When this happened -- which was inconsistent -- the display would keep
some heap allocation across reset which is exactly what we need to avoid.

NULLing the pointer in reconstruct follows what RGBMatrix does, and that
code is a bit more battle-tested anyway.

If I had a motivation for structuring the SharpMemory code differently,
I can no longer recall it.

Testing performed: Ran my complicated calculator program over multiple
iterations without observing signs of heap corruption.

Closes: adafruit#3473
jepler pushed a commit to jepler/circuitpython that referenced this pull request May 6, 2021
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.

2 participants