Last Comment Bug 240131 - NPStream's necko stream ownership and handling seems shady.
: NPStream's necko stream ownership and handling seems shady.
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla15
Assigned To: Josh Aas
: Benjamin Smedberg [:bsmedberg]
Depends on:
  Show dependency treegraph
Reported: 2004-04-09 19:05 PDT by Johnny Stenback (:jst,
Modified: 2012-05-19 06:44 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix v1.0 (4.74 KB, patch)
2012-05-18 07:42 PDT, Josh Aas
jst: review+
Details | Diff | Splinter Review

Description User image Johnny Stenback (:jst, 2004-04-09 19:05:09 PDT
Seems like _destroystream() (in ns4xPlugin.cpp) should do more cleaning up of
the underlying necko streams than it does, like, closing it etc. And the
ownership of the underlying stream is far from obvious, and should be clarified
in the code.
Comment 1 User image Josh Aas 2012-05-13 08:00:36 PDT
From related code comments the primary concern seems to be what we store for stream ndata. I'm working on that code now, taking this.
Comment 2 User image Josh Aas 2012-05-18 07:42:38 PDT
Created attachment 625102 [details] [diff] [review]
fix v1.0

I didn't find any problems during my review of stream cleanup, but there are some things that can be more clear. This patch is largely improving comments, but it also does a little bit of re-organization and makes owning references more clear. It pushes necessary strong references further down in the stream cleanup process to make their necessity clearer.
Comment 3 User image Josh Aas 2012-05-18 13:45:42 PDT
Try server run:
Comment 4 User image Josh Aas 2012-05-18 21:34:55 PDT
pushed to mozilla-inbound
Comment 5 User image Matt Brubeck (:mbrubeck) 2012-05-19 06:44:31 PDT

Note You need to log in before you can comment on or make changes to this bug.