Opened 12 years ago
Closed 12 years ago
#3860 closed Bug (fixed)
memory leak in torrentCallScript
Reported by: | ijuxda | Owned by: | jordan |
---|---|---|---|
Priority: | Normal | Milestone: | 2.20 |
Component: | libtransmission | Version: | 2.13 |
Severity: | Minor | Keywords: | script memory leak |
Cc: |
Description
This function uses tr_strdup* to setup the cmd and env strings, but I cannot for the love of me see where this memory is freed. I'm sort of hoping there are some interesting fork/exec semantics I don't know about, but maybe this is just a mistake.
Change History (7)
comment:1 Changed 12 years ago by charles
- Resolution set to wontfix
- Status changed from new to closed
comment:2 Changed 12 years ago by charles
- Resolution wontfix deleted
- Status changed from closed to reopened
I'm an idiot, fork() makes its own copy of the memory, so the version allocated in the main process should be safe to be freed in the main process.
comment:3 Changed 12 years ago by charles
comment:4 Changed 12 years ago by charles
- Component changed from Transmission to libtransmission
- Milestone changed from None Set to 2.20
- Resolution set to fixed
- Severity changed from Normal to Minor
- Status changed from reopened to closed
seems to be working.
comment:5 Changed 12 years ago by jordan
- Resolution fixed deleted
- Status changed from closed to reopened
comment:6 Changed 12 years ago by jordan
- Owner set to jordan
- Status changed from reopened to new
comment:7 Changed 12 years ago by jordan
- Resolution set to fixed
- Status changed from new to closed
Note: See
TracTickets for help on using
tickets.
Well the libtransmission thread can't free it, because it never knows when the fork is done.
The fork can't free it, because execve never returns except on error.
I dont think we can use alloca() because the stack memory isn't valid once the libtransmission thread exits torrentCallScript().
We /could/ use the forked pid as a key to some static lookup table, and then use the key in onSigCHLD to free the variables, but when I thought about the relative infrequency of torrentCallScript() being invoked, and thought about the pid-lookup scaffolding... leaking a few bytes seemed like a decent tradeoff. :)
If you want to patch it to handle things in onSigCHLD(), please reopen this ticket and I'll use the patch.