|
From: Dave D. <dde...@es...> - 2005-07-08 18:33:11
|
Ethan Merritt <merritt@u.washington.edu> writes: > On Friday 08 July 2005 01:57 am, Dave Denholm wrote: >> That reminds me... there's a couple of memory/cleanup issues been >> mentioned recently, and IIRC, there used to be a problem with >> memory leaks if SIGINT is sent during a plot, since the exact state is >> unknown, and so data structures cannot safely be traversed and freed. > > SIGINT while actually in the eval_plots code must be a rare occurance, > so I'm not sure how much effort we should put into worrying about it. > SIGINT to terminate a "pause for mouse" is much more common, but I can't > see why that would lead to any leaks. > >> allocated blocks are kept in linked lists : say a list of "permanent" >> blocks (eg udf's) > > This is already done. The listhead for udvs is first_udv, the listhead > for labels is first_label, and so on. > I'm talking about the blocks themselves. The memory system maintains a list of all blocks it has given out. Whether the blocks are also linked into other lists (such as lists of plots, or lists of udf, or whatever) is at a higher level. >> and a list of transient blocks (allocated during a >> plot, for example). > Acutally, it's simpler than that. Can have one list of blocks. The reference count is for "permanent" references. Transient blocks have a zero ref. count. Then a gc happens at the end of every transaction, and just reaps everything with 0 reference count. using a block for a udf means that it picks up a ref count, and so the memory manager doesn't reclaim the memory until the udf code drops the reference. Could also store a pointer to an "owner" block. Eg all blocks allocated by a udf would reference that block. When the owner is reaped, all the blocks marked as belonging to it get reaped. The point is just that you do a little more work when you allocate a block (to tell the memory manager what its properties are), but then you don't have to bother finding blocks to free afterwards, because the memory manager is doing the work of tracking stuff. > Hmmm. I think this is also already done, although maybe I am overlooking > a category of allocated space. The major dynamic allocations for a plot > are referenced by elements of the corresponding struct curve_points, > and these structures are themselves linked into a list. > >> At end of a plot, everything in the transient list is reclaimed. > > The previous allocations are freed at the time a new plot is built up. > This should work even if the previous plot was interrupted by SIGINT. > The difference is just that in a gc scheme, at the end of the plot you just tell the memory manager to find and free all transient blocks. You don't have to traverse data structures finding things. Ditto for freeing a udf : you don't have to explicitly look along the actions freeing string literals. You just drop the ref. count on the primary udf block, and the memory manager does all the rest of the work, >> Need to be a little careful with registration of non-trivial objects, >> to make sure things are atomic wrt SIGINT. > > That is indeed an issue in principle (but hard to debug I think!). > > New allocations (udv, whole plots, ...) are added at the end of a > linked list. But this allocate+link operation is not guaranteed > atomic by the current code. I'm not particularly worried by the > possibily of leaking a single newly-allocated block. But if the > SIGINT were to occur at just the wrong time, the final list link > pointer itself could become corrupt. That would show up as a > segfault rather than a memory leak. > > new_element = &(current_end_of_list->next); > new_element = gp_alloc(...); > <<<< SIGINT here is bad news > new_element->next = NULL; > Don't know about you, but I would never write it in that order anyway. > This is fixable, either by careful audit of all the routines that > do list insertion or by modifying gp_alloc to zero out new space > before returning it. > In the proposed scheme, there is exactly one place where the blocks are registered - in the memory manager. So there's only one place to worry about getting it right. All the clients of the memory system just ask for permanent or transient blocks. But anyway, need to be careful when manipulating "permanent" blocks, but for transient blocks, you never need to worry. The memory mananger doesn't follow the clients unsafe links, but only follows its own "safe" links. dd -- Dave Denholm <dde...@es...> http://www.esmertec.com |