misuse of nsISafeOutputStream

RESOLVED INVALID

Status

()

RESOLVED INVALID
13 years ago
13 years ago

People

(Reporter: dwitte, Assigned: dwitte)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

13 years ago
followup from bug 295732 comment 25: all the js code in toolkit that uses nsISOS
does stuff like

var fos =
Components.classes["@mozilla.org/network/safe-file-output-stream;1"].createInstance(Components.interfaces.nsIFileOutputStream);
{...}
fos.finish();

without ever QI'ing the stream to nsISOS. i think that will throw an XPC error,
since XPC doesn't know the instance even supports that iface. which is probably
why ben wrapped all the finish() calls with try/catches.

i don't know the machinery of XPC very well, so if anyone can contradict that,
please do ;)
(Assignee)

Comment 2

13 years ago
Created attachment 187695 [details] [diff] [review]
use qi

something like this. timeless, mind taking a look?
(Assignee)

Updated

13 years ago
Attachment #187695 - Flags: review?(timeless)

Comment 3

13 years ago
Comment on attachment 187695 [details] [diff] [review]
use qi

-  if (stream instanceof Components.interfaces.nsISafeOutputStream)
-    stream.finish();
-  else
-    stream.close();

unless i'm mistaken, this code adds properties from nsISafeOutputStream onto
stream.  instanceof is just another way of doing QueryInterface, so i don't see
how this patch changes anything.

Comment 4

13 years ago
Comment on attachment 187695 [details] [diff] [review]
use qi

>-  if (stream instanceof Components.interfaces.nsISafeOutputStream)
in general this causes an implicit QI to the interface and stream is annotated
to include a pointer to the interface, flattening then enables you to call
methods of that interface.

>+    var sos = fos.QueryInterface(Components.interfaces.nsISafeOutputStream);

(note that the code you're replacing calls close() in the else case, your new
code doesn't do anything.) i don't believe you've found the right problem
source.

Comment 5

13 years ago
Here's a little xpcshell session to demonstrate what I'm talking about:

js> var f =
Components.classes["@mozilla.org/network/safe-file-output-stream;1"].createInstance();
js> f
[xpconnect wrapped nsISupports @ 0x8167a60 (native @ 0x8167a28)]
js> for (p in f) dump(p + "\n");
QueryInterface
js> f instanceof Components.interfaces.nsISafeOutputStream;
true
js> for (p in f) dump(p + "\n");
QueryInterface
finish
Comment on attachment 187695 [details] [diff] [review]
use qi

-  if (stream instanceof Components.interfaces.nsISafeOutputStream)

this is a QI in disguise, I don't think this patch is needed.
mozillafly - are you still able to reproduce this error? As I recall, it
depended on the version of gcc you were using.
(Assignee)

Comment 8

13 years ago
(In reply to comment #6)
> this is a QI in disguise, I don't think this patch is needed.

yeap, gotcha... thanks.

agreed that the problem must be something else, no need for this bug.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → INVALID

Comment 9

13 years ago
(In reply to comment #7)
> mozillafly - are you still able to reproduce this error? As I recall, it
> depended on the version of gcc you were using.

I have recently done builds with gcc 3.3.6 and gcc 4.0.1 and am not able to
reproduce the error I was seeing before.  
You need to log in before you can comment on or make changes to this bug.