Plug hole in struct event lifecycle; should fix bug 10503

StopWatchingFileDescriptor needs to free struct event.
Also, there's no point in using scoped_ptr for event_ anymore,
so removed that.
Should fix http://crbug.com/10503 "Crash in network layer"



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=14233

[dank, 2009-04-21 19:55:25]

How's this look?  Passes net and base under valgrind now.

[jeremy, 2009-04-21 20:03:51]

http://codereview.chromium.org/87045/diff/1/2
File base/message_loop_unittest.cc (right):

http://codereview.chromium.org/87045/diff/1/2#newcode1385
Line 1385: #if defined(OS_LINUX)
OS_POSIX?  Is there anything that prevents this test from running on OS X?

http://codereview.chromium.org/87045/diff/1/2#newcode1387
Line 1387: #include "base/message_pump_libevent.h"
Move include to top of file.

http://codereview.chromium.org/87045/diff/1/2#newcode1402
Line 1402: }
// namespace

http://codereview.chromium.org/87045/diff/1/3
File base/message_pump_libevent.cc (right):

http://codereview.chromium.org/87045/diff/1/3#newcode58
Line 58: return rv;
nit: return (rv == 0)

[dank, 2009-04-21 23:22:02]

Done, I think.  Also fixed a leak (assigning to NULL was
about right after all), and added a paragraph about struct event's
life cycle. 

(Is there any reason to use scoped_ptr<> for event_ anymore?
It seems that its lifetime is nearly completely manually managed...)

[dank, 2009-04-22 17:50:13]

Cleaned up new comment (which had glaring mistakes),
moved away from scoped_ptr since it no longer makes sense.

[jeremy, 2009-04-22 17:56:15]

http://codereview.chromium.org/87045/diff/2001/2003
File base/message_pump_libevent.cc (right):

http://codereview.chromium.org/87045/diff/2001/2003#newcode175
Line 175: event_del(evt);
delete evt?

Maybe keep using scoped_ptr just in this function?

[dank, 2009-04-22 18:23:46]

Done.

[jeremy, 2009-04-22 19:41:25]

LGTM