Addresses open file handles when parse errors occur in include files (Issue #5942) #6050
Conversation
|
An addition in msautotest might be useful |
|
A specific test would need to take the form of something that would show the number of open file handles after a process closed. I'm not sure how that would be done or how it could be included in the test suite. |
I had presumed that the CI config that runs with Address Sanitizer enabled (PHP_7.2_WITH_ASAN) would warn about unclosed files as a memory leak, but on a small test case, I see this isn't the case unfortunately. Valgrind can see this with --leak-check=full --show-leak-kinds=all however However I think there's still a merit in having such a test case: at least this makes sure that the error code paths are hit and don't cause crash (I'm sure they don't now :-), but it might if someone later touches it) |
|
I can certainly create a couple of scenarios that generate parse errors 0, 1 and 2 includes deep.
From: Even Rouault [mailto:notifications@github.com]
Sent: Thursday, April 16, 2020 5:08 PM
To: mapserver/mapserver <mapserver@noreply.github.com>
Cc: Lime, Steve D (MNIT) <steve.lime@state.mn.us>; Author <author@noreply.github.com>
Subject: Re: [mapserver/mapserver] Addresses open file handles when parse errors occur in include files (Issue #5942) (#6050)
This message may be from an external email source.
Do not select links or open attachments unless verified. Report all suspicious emails to Minnesota IT Services Security Operations Center.
…________________________________
A specific test would need to take the form of something that would show the number of open file handles after a process closed
I had presumed that the CI config that runs with Address Sanitizer enabled (PHP_7.2_WITH_ASAN) would warn about unclosed files as a memory leak, but on a small test case, I see this isn't the case unfortunately. Valgrind can see this with --leak-check=full --show-leak-kinds=all however
==27735== 552 bytes in 1 blocks are still reachable in loss record 1 of 1
==27735== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27735== by 0x4EA7CDC: __fopen_internal (iofopen.c:69)
==27735== by 0x40053C: main (leak.c:4)
However I think there's still a merit in having such a test case: at least this makes sure that the error code paths are hit and don't cause crash (I'm sure they don't now :-), but it might if someone later touches it)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://gcc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmapserver%2Fmapserver%2Fpull%2F6050%23issuecomment-614921955&data=02%7C01%7Csteve.lime%40state.mn.us%7Cfddfb8034e4e47c3805908d7e252a88b%7Ceb14b04624c445198f26b89c2159828c%7C0%7C0%7C637226716950720578&sdata=ua24rqmVXcs7bJ5nj95wCnDAnnNvk6cLRBOd%2FhQujqY%3D&reserved=0>, or unsubscribe<https://gcc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAMGN63UBATFNSRRINBLQXLRM56UZANCNFSM4MIWJZSA&data=02%7C01%7Csteve.lime%40state.mn.us%7Cfddfb8034e4e47c3805908d7e252a88b%7Ceb14b04624c445198f26b89c2159828c%7C0%7C0%7C637226716950720578&sdata=Bxlf7BlseykPVhO3o9jXHHZyabf5rcluTWj8OcPZNZs%3D&reserved=0>.
|
|
Bump, @dmorissette or @alexbrault, do you have time to test this fix? --Steve |
|
No response on testing, but am merging this now... |
No description provided.