Comment 4 for bug 787736

Revision history for this message
JKL (jkl102001) wrote : Re: memory leak in rebuild

I'm not sure what you are saying, but looking back at it now, I think the patch I wrote was overcomplicated. The design can be simplified considerably, and in doing so two additional issues can be addressed.

The first issue has to do with scheduling. The existing design cancels the prior scheduled rebuild of the menu, which means that if rebuild ends up getting called at short intervals over a long period of time, the menu will never end up getting rebuilt.

I think it is better to simply check whether a rebuild is already scheduled, and if so, do nothing. That way you ensure that the rebuild doesn't happen too often, but you don't end up postponing it indefinitely in the worst case.

The second issue has to do with the life cycle of the hash table. As written, it is difficult to understand when the hash table is created and destroyed, and why it is safe to destroy it. In particular, we must prove there are no pending timeouts when we destroy the table, because we don't want to free the RebuildData out from under a pending timeout.

I think it is better to make the hash table a private member of the bridge rather than a global variable. That way it is possible to write a very simple argument showing why it is safe to destroy the hash table when the bridge is finalized.

I checked out your branch and wrote a patch on top of it demonstrating what I have in mind.