Opened 10 years ago

Closed 10 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 10 years ago by charles

  • Resolution set to wontfix
  • Status changed from new to closed

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.

comment:2 Changed 10 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 10 years ago by charles

comment:4 Changed 10 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 10 years ago by jordan

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:6 Changed 10 years ago by jordan

  • Owner set to jordan
  • Status changed from reopened to new

comment:7 Changed 10 years ago by jordan

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.