Bug #72213 [ReO->Csd]: Finally leaks on nested exceptions

Edit report at https://bugs.php.net/bug.php?id=72213&edit=1 ID: 72213 Updated by: laruence@php.net Reported by: nikic@php.net Summary: Finally leaks on nested exceptions -Status: Re-Opened +Status: Closed Type: Bug Package: Scripting Engine problem PHP Version: master-Git-2016-05-13 (Git) Block user comment: N Private report: N New Comment: Automatic comment on behalf of laruence@gmail.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=fad91468dbf48401ac3fc719dad3ce9f93e1a378 Log: Revert &quot;Fixed Bug #72213 (Finally leaks on nested exceptions)&quot; Previous Comments: ------------------------------------------------------------------------ [2016-05-17 12:28:06] laruence@php.net you are right. this fix is not complete right, I reverted. thanks ------------------------------------------------------------------------ [2016-05-17 11:45:36] nikic@php.net This patch does not look correct to me... First, it breaks this code: <?php function test() { try { throw new Exception(1); } finally { try { try { throw new Exception(2); } finally {} } catch (Exception $e) {} } } try { test(); } catch (Exception $e) { echo "caught {$e->getMessage()}\n"; } Previously this correctly threw Exception(1), matching Java. Now nothing is thrown instead. Second, it only avoids the leak for one nesting level. This will still leak: <?php function test() { try { throw new Exception('a'); } finally { try { } finally { try { throw new Exception('b'); } finally { } } } } try { test(); } catch (Exception $e) { echo "caught {$e->getMessage()}\n"; } ------------------------------------------------------------------------ [2016-05-17 07:33:10] laruence@php.net Automatic comment on behalf of laruence@gmail.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=5037ebf058bdc94e4426240a6d35fcf427d2eee2 Log: Fixed Bug #72213 (Finally leaks on nested exceptions) ------------------------------------------------------------------------ [2016-05-16 15:37:40] laruence@php.net The following patch has been added/updated: Patch Name: bug72213.diff Revision: 1463413056 URL: https://bugs.php.net/patch-display.php?bug=72213&patch=bug72213.diff&revision=1463413056 ------------------------------------------------------------------------ [2016-05-13 14:55:30] nikic@php.net Description: ------------ Attached script causes a leak: caught b [Fri May 13 16:51:34 2016] Script: '/home/nikic/php-src/t275.php' /home/nikic/php-src/Zend/zend_objects.c(174) : Freeing 0x7F6FB46740C0 (152 bytes), script=/home/nikic/php-src/t275.php === Total 1 memory leaks detected === The leak is the exception backed up in the outer fast_call, which needs to be discarded. Test script: --------------- <?php function test() { try { throw new Exception('a'); } finally { try { throw new Exception('b'); } finally { } } } try { test(); } catch (Exception $e) { echo "caught {$e->getMessage()}\n"; } ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=72213&edit=1