device-os icon indicating copy to clipboard operation
device-os copied to clipboard

Avoid freeing memory in detachInterrupt

Open metcalf opened this issue 2 years ago • 1 comments

Problem

When attachHandler is called with a function pointer (or a pointer to a member function and instance), it allocates a std::function. When detachHandler is then called on the same pin, it calls delete to free that memory. That causes a heap error if you try to call detachHandler on this pin from an ISR. The same is true for attaching and detaching system interrupts.

Solution

This PR removes the deletes and leaves the memory allocated to be re-used the next time attach* is called. This prevents the application from reclaiming the memory std::function is allocating on the heap, but I suspect that's OK since there can be no more allocations than the number of pins.

I think the alternative is to update the documentation to specify when detachInterrupt can be called. It's probably still worth updating the docs to specify that attachInterrupt and attachSystemInterrupt cannot be called from an ISR when used with a function pointer / class+member.

Steps to Test

Wrote and ran an integration test on my Photon.

Example App

Deferring to the integration test, but can write something if necessary.

References

The delete in detachSystemInterrupt was added in #951. I think the primary problem fixed in that PR was an unbounded memory leak repeatedly calling attachSystemInterrupt.


Completeness

  • [x] User is totes amazing for contributing!
  • [x] Contributor has signed CLA (Info here)
  • [ ] Problem and Solution clearly stated
  • [ ] Run unit/integration/application tests on device
  • [ ] Added documentation
  • [ ] Added to CHANGELOG.md after merging (add links to docs and issues)

metcalf avatar Sep 14 '21 16:09 metcalf

Checking in on this since it's blocking me from using Cloud Flash. Is this likely to get merged or should I rewrite my application to avoid this issue?

metcalf avatar Nov 19 '21 22:11 metcalf