dovecot - Jun 2010 - Patch to fix leak in imap_refresh_proctitle in beta[5, 6]

The "imap" process of dovecot-2.0.beta[5,6] grows very large (I impose
no system limits), e.g. exceeding 4.8GB on a 64-bit system.  These messages
appear in the logs:
Warning: Growing pool 'imap client' with: 2048
Warning: Growing pool 'Cache fields' with: 2048
Warning: Growing data stack with: 32768
Warning: Growing data stack with: 65536
Warning: Growing data stack with: 131072
[...]
Warning: Growing data stack with: 1073741824
Warning: Growing data stack with: 2147483648
Warning: Growing data stack with: 4294967296

Every time the data stack grows, the backtrace ends with either:
2   libdovecot.0.dylib                  0x00000001059a35c3
pool_data_stack_realloc + 72 ->
3   libdovecot.0.dylib                  0x0000000105994d59 buffer_alloc + 59
->
4   libdovecot.0.dylib                  0x0000000105994ef9 buffer_check_limits +
127 ->
5   libdovecot.0.dylib                  0x00000001059950e8 buffer_append + 38
->
6   imap                                0x0000000105867333
imap_refresh_proctitle + 218 ->
7   imap                                0x000000010585f18e client_command_input
+ 190 ->
[...]

or

2   libdovecot.0.dylib                  0x00000001059a35c3
pool_data_stack_realloc + 72 ->
3   libdovecot.0.dylib                  0x0000000105994d59 buffer_alloc + 59
->
4   libdovecot.0.dylib                  0x0000000105994ef9 buffer_check_limits +
127 ->
5   libdovecot.0.dylib                  0x00000001059950e8 buffer_append + 38
->
6   imap                                0x0000000105867333
imap_refresh_proctitle + 218 ->
7   imap                                0x00000001058666ce cmd_sync_continue +
199 ->
[...]

Here is a patch to fix it:
--- a/src/imap/main.c	(beta6)
+++ b/src/imap/main.c	(working copy)
@@ -39,11 +39,12 @@
 #define IMAP_PROCTITLE_PREFERRED_LEN 80
 	struct client *client;
 	struct client_command_context *cmd;
-	string_t *title = t_str_new(128);
+	string_t *title;
 
 	if (!verbose_proctitle)
 		return;
 
+	title = str_new(default_pool, 128);
 	str_append_c(title, '[');
 	switch (imap_client_count) {
 	case 0:
@@ -72,6 +73,7 @@
 	}
 	str_append_c(title, ']');
 	process_title_set(str_c(title));
+	str_free(&title);
 }
 
 static void client_kill_idle(struct client *client)

And perhaps there should be a warning when data is allocated out of the
primordial data stack (as opposed to a nested one)?

On Mon, 2010-06-14 at 17:18 -0500, Mike Abbott wrote:
> 6   imap                                0x0000000105867333
imap_refresh_proctitle + 218 ->
> 7   imap                                0x000000010585f18e
client_command_input + 190 ->
Looks ok..
> 6   imap                                0x0000000105867333
imap_refresh_proctitle + 218 ->
> 7   imap                                0x00000001058666ce
cmd_sync_continue + 199 ->
But how does this happen? Did it optimize away some functions or have
you added more imap_refresh_proctitle() calls?
> And perhaps there should be a warning when data is allocated out of
> the primordial data stack (as opposed to a nested one)?
But it's not. It always frees the memory when returning back to ioloop.
I could understand if it wasted a few kilobytes of memory, but how do
you manage to make it call imap_refresh_proctitle() millions of times
without returning to ioloop?