Hi Anatol, On Sat, Jan 30, 2016 at 7:15 PM, Anatol Belski <a...@php.net> wrote: >> -----Original Message----- >> From: yohg...@gmail.com [mailto:yohg...@gmail.com] On Behalf Of Yasuo >> Ohgaki >> Sent: Saturday, January 30, 2016 4:13 AM >> To: Anatol Belski <a...@php.net> >> Cc: php-...@lists.php.net >> Subject: Re: [PHP-CVS] com php-src: fix leak in 5.6: ext/session/mod_files.c >> >> On Sat, Jan 30, 2016 at 11:03 AM, Yasuo Ohgaki <yohg...@ohgaki.net> wrote: >> > I'm just curious which case causes memory leak. >> > php_rshutdown_globals() is always called at shutdown and s_close() >> > should clean them up. >> > http://lxr.php.net/xref/PHP_5_6/ext/session/session.c#104 >> > >> > Thank you. >> >> s/php_rshutdown_globals()/php_rshutdown_session_globals()/ >> > Thanks for the check. The actual reproduce case in #69111 was causing crash > in 5.6 only, so I made just a simple NULL check. So far it was fine on Debian > stable and old stable, but then travis showed that there was a memory leak on > the session data. Furthermore, despite the code base in 7.0 is somewhat > different, the memory leak is present as well. The basic situation is the > same - when the session id is invalid, it refuses the read/write operation > and seems that at some place the proper freeing doesn't happen. > > As valgrind on my dev environment was not showing any issue, I attempted to > analyze the code flow, then commit and see what travis tells. But as I don't > reproduce this leak locally, it's impossible to properly debug the case. So > finally I've reverted my attempts to fix the memory leak and set the test to > XFAIL until me or someone else can reproduce and debug it locally :( I'll > probably need to setup some more recent OS version for that, which I have no > time to. If it's reproducable for someone, please go ahead.
Thank you for the explanation. I'll look into this. Session module should manage state precisely and it makes things complicated. I guess there is/are missing state management in it. Current code is not explicit about state management. I may try to clean them up and rewrite it for PHP8. In the meantime, I'll add many asserts and comments for better code readability and future rewrite. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
