88 memory leak in imagechannel - Githubissues

EmilDohne / PhotoshopAPI

A modern and performant C++20 read/write parser of Photoshop Files (*.psd and *.psb) with fully fledged Python bindings hosted on PyPi

https://photoshopapi.readthedocs.io/en/latest/

BSD 3-Clause "New" or "Revised" License

100 stars 11 forks source link

88 memory leak in imagechannel #89

Closed EmilDohne closed 3 months ago

EmilDohne commented 3 months ago

While fixing the issue at hand I also went ahead and refactored some of the older parts of the codebase which had questionable code decisions leading to some technical debt. Especially the polymorphism of the ImageChannel struct to allow for templates. This was simplified down to just having some constructors be templated which alleviated this problem.

We now also always free the data on destruction and have changed all references to ImageChannel to hold a unique ptr without any weird downcasting

EmilDohne commented 3 months ago

The segfaults are concerning, so are the test failures in completely unrelated areas. I will have to investigate why tomorrow

EmilDohne commented 3 months ago

It looks like these issues didnt appear today but instead have already been there for quite a while but due to the status being "Success" even when some test cases fail this appears to have been the reason why they flew under the radar for so long.

Overall this issue must then be approached in 4 steps:

Figure out why test failures do not cause a CTest failure
Figure out why the tests fail in the first place
Implement Clang as an additional test suite
Fix python segfaults

EmilDohne commented 3 months ago

It looks like 1 was caused by not explicitly returning result from doctest but only on context.shouldExit() which was leading to it implicitly returning 0

EmilDohne commented 3 months ago

Looks like all the issues have been ironed out except for getting clang to properly link the executables but I will look into that at a later point

EmilDohne commented 3 months ago

Turned out to be quite a large PR with a lot of changes but I managed to fix some lingering issues and improved the code quality by a decent bit so I am happy with it.

This shouldnt have any API effects since we only touched implementation stuff