On Sat, 19 Oct 2013, David T. Lewis wrote:
Hi Levente,
I opened a Mantis issue to make sure this does not get overlooked:
Thanks. I've attached the file with the changes.
Do you now have an updated sqUnixOpenSSL.c that fixes the memory leak problem? If so can you please post the file here (or attach it to the Mantis report)? I understand the changes from your original message but I'm not clear if more work needs to be done related to "the BIO_free_all calls are not needed in general".
The sqUnixOpenSSL.c file is identical on trunk and oscog, so we can apply your fix to both branches.
I made some changes, and it seems like the leakage is gone. I only ran a short test of a few thousand connections, but I didn't experience any increase in memory usage with the new code. You can find the modified file here: http://leves.web.elte.hu/squeak/sqUnixOpenSSL.c
Both the cmake (for the interpreter) and the automake (for Cog) configs will have to be created/updated to be able to build a functional plugin.
Levente
Thanks! Dave
On Thu, Oct 17, 2013 at 11:50:32PM +0200, Levente Uzonyi wrote:
I tried to set up a build environment to be able to build a functional SqueakSSL plugin and test this idea. The only barrier was that the plugin is not linked to libssl nor to libcrypto, so it won't do anything. You can use ldd to check if this is the case with your plugin too:
$ ldd SqueakSSL linux-gate.so.1 => (0xf7763000) libc.so.6 => /lib/i386-linux-gnu/libc.so.6 (0xf7595000) /lib/ld-linux.so.2 (0xf7764000)
This is currently the case with Eliot's build. To make the plugin work I added -lssl and -lcrypto manually to the Makefile. I have no idea how to add it to autoconf, so someone else will have to do that. With those flags ldd will show something like this:
$ ldd SqueakSSL linux-gate.so.1 => (0xf77c0000) libssl.so.1.0.0 => /lib/i386-linux-gnu/libssl.so.1.0.0 (0xf7760000) libcrypto.so.1.0.0 => /lib/i386-linux-gnu/libcrypto.so.1.0.0 (0xf75b5000) libc.so.6 => /lib/i386-linux-gnu/libc.so.6 (0xf740b000) libdl.so.2 => /lib/i386-linux-gnu/libdl.so.2 (0xf7406000) libz.so.1 => /lib/i386-linux-gnu/libz.so.1 (0xf73f0000) /lib/ld-linux.so.2 (0xf77c1000)
It quickly turned out that the BIO_free_all calls are not needed in general, only when the ssl structure doesn't get allocated, because SSL_free frees both bioRead and bioWrite.
Levente
On Thu, 17 Oct 2013, Levente Uzonyi wrote:
Seems like the end of the message got lost. So here's the full message again:
Hello,
we've been experiencing memory leakage in long running Squeak images using SqueakSSL. After a bit of monitoring I found that 132 bytes get leaked for each https request done from the image. After a bit of code review, I've probably found the culprit, and another potential source of memory leak. For the reference, the source file this mail is about is http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/branches/Cog/platforms/unix/pl...
The main memory leak is in sqDestroySSL function (starting on line 117), which doesn't free the bioRead and bioWrite variables (allocated by sqCreateSSL on line 98-99) of the ssl object. My suggested solution is to insert the following two lines before line 132:
BIO_free_all(ssl->bioRead); BIO_free_all(ssl->bioWrite);The other potential source of memory leak is sqSetStringPropertySSL (starting on line 381). It allocates a chunk of memory on line 389, but doesn't use nor free it, if the propID argument is not SQSSL_PROP_CERTNAME. My suggested solution is to insert the following line after line 396:
if(property) free(property);Note that I haven't tested any of these, but I hope someone who is more into VM building right now will try them.
Cheers, Levente