Download raw body.
Plug memory leak in some libexecs
On Sun, May 04, 2025 at 11:59:20PM +0000, Kyle Ackerman wrote:
> There is currently an imsg memory leak in a couple of our libexecs.
> This occur when we these particular libexecs get an imsg of type
> GOT_IMSG_STOP. Which thet then just call break, not freeing the
> imsg it just received. Of the libexecs I noticed having this
> behavior, we can jump to "done" to cleanup when we get
> GOT_IMSG_STOP. This would also introduce the need for a variable
> (which I called "finished") that we can set to determine whether
> we need to break out of the main loop.
ok by me, thanks!
> diff /home/kyle/src/got
> path + /home/kyle/src/got
> commit - 4492e47bc914650ecd587fcc94010ae0373ab91b
> blob - d23947e452d72ebf846600e7bf03c9ea2d1d2bd0
> file + libexec/got-read-blob/got-read-blob.c
> --- libexec/got-read-blob/got-read-blob.c
> +++ libexec/got-read-blob/got-read-blob.c
> @@ -78,7 +78,7 @@ main(int argc, char *argv[])
> for (;;) {
> struct imsg imsg, imsg_outfd;
> FILE *f = NULL;
> - int fd = -1, outfd = -1;
> + int fd = -1, outfd = -1, finished = 0;
> size_t size;
> struct got_object *obj = NULL;
> uint8_t *buf = NULL;
> @@ -102,9 +102,12 @@ main(int argc, char *argv[])
> break;
> }
>
> - if (imsg.hdr.type == GOT_IMSG_STOP)
> - break;
> + if (imsg.hdr.type == GOT_IMSG_STOP) {
> + finished = 1;
> + goto done;
> + }
>
> +
> if (imsg.hdr.type != GOT_IMSG_BLOB_REQUEST) {
> err = got_error(GOT_ERR_PRIVSEP_MSG);
> goto done;
> @@ -202,9 +205,12 @@ done:
>
> imsg_free(&imsg);
> imsg_free(&imsg_outfd);
> - if (obj)
> + if (obj) {
> got_object_close(obj);
> - if (err)
> + obj = NULL;
> + }
> +
> + if (err || finished)
> break;
> }
>
> commit - 4492e47bc914650ecd587fcc94010ae0373ab91b
> blob - a37151e9c26c717e0134c3112302bbaa24cbefb6
> file + libexec/got-read-gitconfig/got-read-gitconfig.c
> --- libexec/got-read-gitconfig/got-read-gitconfig.c
> +++ libexec/got-read-gitconfig/got-read-gitconfig.c
> @@ -365,7 +365,7 @@ main(int argc, char *argv[])
>
> for (;;) {
> struct imsg imsg;
> - int fd = -1;
> + int fd = -1, finished = 0;
>
> memset(&imsg, 0, sizeof(imsg));
>
> @@ -381,8 +381,10 @@ main(int argc, char *argv[])
> break;
> }
>
> - if (imsg.hdr.type == GOT_IMSG_STOP)
> - break;
> + if (imsg.hdr.type == GOT_IMSG_STOP) {
> + finished = 1;
> + goto done;
> + }
>
> switch (imsg.hdr.type) {
> case GOT_IMSG_GITCONFIG_PARSE_REQUEST:
> @@ -427,13 +429,14 @@ main(int argc, char *argv[])
> break;
> }
>
> + done:
> if (fd != -1) {
> if (close(fd) == -1 && err == NULL)
> err = got_error_from_errno("close");
> }
>
> imsg_free(&imsg);
> - if (err)
> + if (err || finished)
> break;
> }
>
> commit - 4492e47bc914650ecd587fcc94010ae0373ab91b
> blob - c7cf9727a6e3f4be0a7d3e1b689f108ee2a99d9c
> file + libexec/got-read-gotconfig/got-read-gotconfig.c
> --- libexec/got-read-gotconfig/got-read-gotconfig.c
> +++ libexec/got-read-gotconfig/got-read-gotconfig.c
> @@ -513,7 +513,7 @@ main(int argc, char *argv[])
>
> for (;;) {
> struct imsg imsg;
> - int fd = -1;
> + int fd = -1, finished = 0;
>
> memset(&imsg, 0, sizeof(imsg));
>
> @@ -529,8 +529,10 @@ main(int argc, char *argv[])
> break;
> }
>
> - if (imsg.hdr.type == GOT_IMSG_STOP)
> - break;
> + if (imsg.hdr.type == GOT_IMSG_STOP) {
> + finished = 1;
> + goto done;
> + }
>
> switch (imsg.hdr.type) {
> case GOT_IMSG_GOTCONFIG_PARSE_REQUEST:
> @@ -598,14 +600,14 @@ main(int argc, char *argv[])
> err = got_error(GOT_ERR_PRIVSEP_MSG);
> break;
> }
> -
> + done:
> if (fd != -1) {
> if (close(fd) == -1 && err == NULL)
> err = got_error_from_errno("close");
> }
>
> imsg_free(&imsg);
> - if (err)
> + if (err || finished)
> break;
> }
>
> commit - 4492e47bc914650ecd587fcc94010ae0373ab91b
> blob - cacaca8f4f78e8ba52a4e42ad12a6e42e9c6d950
> file + libexec/got-read-object/got-read-object.c
> --- libexec/got-read-object/got-read-object.c
> +++ libexec/got-read-object/got-read-object.c
> @@ -116,7 +116,7 @@ main(int argc, char *argv[])
> #endif
>
> for (;;) {
> - int fd = -1, outfd = -1;
> + int fd = -1, outfd = -1, finished = 0;
>
> if (sigint_received) {
> err = got_error(GOT_ERR_CANCELLED);
> @@ -130,8 +130,10 @@ main(int argc, char *argv[])
> break;
> }
>
> - if (imsg.hdr.type == GOT_IMSG_STOP)
> - break;
> + if (imsg.hdr.type == GOT_IMSG_STOP) {
> + finished = 1;
> + goto done;
> + }
>
> if (imsg.hdr.type != GOT_IMSG_OBJECT_REQUEST &&
> imsg.hdr.type != GOT_IMSG_RAW_OBJECT_REQUEST) {
> @@ -203,9 +205,11 @@ done:
> if (fd != -1 && close(fd) == -1 && err == NULL)
> err = got_error_from_errno("close");
> imsg_free(&imsg);
> - if (obj)
> + if (obj) {
> got_object_close(obj);
> - if (err)
> + obj = NULL;
> + }
> + if (err || finished)
> break;
> }
>
>
> Thoughts/Comments/Suggestions/oks?
>
>
Plug memory leak in some libexecs