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

Fix memleak in tjunittest.c #61

Closed
wants to merge 1 commit into from

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented Feb 25, 2016

While running tests under ASan, I ran into a memleak in tjunittest.c.

@dcommander, in order to test some of my modifications, I'm setting up CI on my repo (travis-ci for now). I don't know if you're using something on your end or if that's something you'd be interested in, in which case I'd be happy to make a PR to add the proper config file.

@dcommander
Copy link
Member

I am interested in continuous integration, but I haven't had time to dig into it. Ideally, I would want a continuous integration server to take the place of my existing "official" build processes, which are only semi-automatic. But the official build process is pretty specific (refer to https://github.com/libjpeg-turbo/buildscripts), because it seeks to retain as much backward compatibility with older O/S's as possible. Travis is unfortunately running too new of an O/S (Ubuntu 12.04) to achieve that. If I built on that platform, then the resulting binaries wouldn't work on RHEL 5, which I need to continue supporting for another year, and SuSE 11, which I need to continue supporting until probably 2019. libjpeg-turbo feeds into a couple of other projects (VirtualGL and TurboVNC) that I actively support for large corporations and universities and such who are still running those older platforms. My policy is to support RHEL releases until the end of the Production Phase, Ubuntu LTS releases until the end of maintenance updates, and SuSE Linux Enterprise until the end of General Support.

Thus, Travis would really only be useful as a way of validating that nothing is broken-- but only on Linux and OS X. There is some benefit to that-- mainly in continuously testing the more esoteric code paths that don't receive a lot of attention (12-bit support, etc.), but it would also be missing any tests for non-x86 architectures, Windows (which uses a completely different build system), etc. The matrix of support for libjpeg-turbo is now quite large, and the incremental benefit of continuously integrating just a couple of those platforms currently isn't worth the pain of getting it up and running. libjpeg-turbo tends to advance more in spurts rather than continuously, and the frequency of those advances is slowing as people become more and more satisfied with it. Architecturally, I expect that we'll reach a level of maturity within the next 1-2 years where very little else needs to be done to it except for perhaps adding new SIMD optimizations. libjpeg-turbo 1.6 will largely be built around AVX2 support and your SIMD-accelerated progressive encoding contributions, with the addition of some minor enhancements to the TurboJPEG API. After that, there's really nothing on the horizon, except for ongoing maintenance.

@mayeut
Copy link
Contributor Author

mayeut commented Feb 29, 2016

From what you're saying, I'm understanding that the benefits of setting up CI for libjpeg-turbo is not worth the effort & I'll stop my investigations to what's written below (however I personally think a minimum CI environment is always worth setting up, even if it's not the same environment as the official one. Mostly, this can triage some PR quite easily, must build - multiple platforms can be done here -, make test OK on x86/x64, ASan checks, Coverage).

I looked at the official build process to see if I could do something with available open source ci services for native code that integrate with GitHub (to my knowledge, travis-ci & appveyor). Not much can be done except for Linux.

Regarding the needed level of support for linux (glibc 2.5), there's a docker image for CentOS 5 and I was able to get something built using the official process (https://travis-ci.org/mayeut/libjpeg-turbo/builds/112412103, uses docker image mayeut/ljtbuilder and slightly modified scripts to allow sources already unpacked in a specified directory to use travis-ci git clone).

Mac OS X / iOS builds can't be done on travis-ci following the official build procedure. The oldest Xcode version supported is Xcode 6.1 (running on 10.9.x). There's also a problem running 32 bits JVM now that all JVMs are 64 bits on recent OS X versions.

Windows build can't be done on appveyor using Visual Studio 2010 for 64 bits (The Express edition which is used on appveyor only allows 32 bits builds). They also have VS 2013/2015 community editions which allow for 64 bits builds (mingw builds should't be a problem).

@dcommander
Copy link
Member

OK, well, if we can do official Windows and Linux builds, then that seems beneficial enough to justify the feature. If you're willing to do the work, then I'd say go ahead and finish it as a proof of concept, and I can play with it and tweak it to my needs. I just mainly needed someone to get me started, because I don't have time to dig into all of the specifics of travis-ci and AppVeyor.

@dcommander
Copy link
Member

Ugh. Docker isn't a free service, so unfortunately that would put a wrench in that idea, unless they have a free option I'm not noticing. Maybe you're right and it's best to use this CI stuff more as a way of validating the correctness of the code instead of generating official builds.

@mayeut
Copy link
Contributor Author

mayeut commented Mar 1, 2016

Docker is Open-Source software. It's available freely on Linux through apt/yum install on recent distributions. It also runs quite well on Windows / Mac OS X using VirtualBox to run a lightweight Linux (and the docker tool manages all aspects of running/communicating a/with a VM so that the CLI is the same than on Linux directly on the host).

@dcommander
Copy link
Member

Yes, but that would require that I have my own hosted Linux server.

@mayeut
Copy link
Contributor Author

mayeut commented Mar 1, 2016

Nope, docker can run on travis-ci.org workers. In the link I mentioned above https://travis-ci.org/mayeut/libjpeg-turbo/builds/112412103, all is running on travis-ci.org infrastructure. The docker image is hosted on hub.docker.com. This is all free for OpenSource software.

@dcommander
Copy link
Member

Cool. If you're willing to do the work to develop a proof of concept for this, I will certainly spend the time necessary to integrate it. I usually learn new systems by looking at the work of others, so being able to look at your scripts will help me develop an understanding of how travis-ci and Docker work.

@mayeut mayeut deleted the tjunittest-memleak branch March 5, 2016 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants