68392 - Cache is defunct after branch landing

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect, P1)

Description

[Scott MacGregor]

I'm no longer able to open mail attachments for my imap account. 

Investigating.

Comment 1

[Scott MacGregor]

Doug, I think the problems coming from the memory cache. It appears to be broken
after your changes. 

any call to AsyncRead on the memory cache entry channel is returning an error!

*doh*

Comment 2

[Scott MacGregor]

I think many of the mailnews problems I'm running into (news is pretty broken
too) is because the memory cache doesn't work. =(

Maybe you can help me understand how this is supposed to work Doug/Gordon:
We call nsMemCacheTransport::AsyncRead to start reading data from the cache
entry. At this time, the cache channel's mCurrentReadRequest is NULL. It calls
AsyncReadStreamAdaptor::AsyncRead passing in mCurrentReadRequest as an out
parameter which should be filled in by AsyncReadStreamAdaptor.

Unfortunately AsyncStreamAdaptor starts off with the following check:
        if (!mChannel->mCurrentReadRequest)
            return NS_ERROR_FAILURE;
But mChannel's mCurrentReadRequest is supposed to be filled in by
AsyncStreamAdaptor so of course it's going to be null.

On top of that asyncstream adaptor doesn't fill the nsIRequest argument we pass
to it.

This is definetly why I can't open up mail attachments. Because we are finding
the imap url in the memory cache.

Comment 3

[Chris McAfee]

adding smoketest to get this on tbox radar

Comment 4

[Scott MacGregor]

I don't believe there is a "quick fix" for getting the memory cache working
again. Maybe dougt or gordon can show me something I missed.

nsMemCacheChannel used to support nsIChannel. When a consumer called AsyncRead
on the cache entry, we got a nsIRequest back in all the nsIStreamListener
callbacks. This request object could be QI'ed for a nsIChannel which we leverage.

After the necko landing the mem cache channel is now a nsITransport and no
longer inherits from nsIChannel. So there is no channel object to pass back (via
a nsIRequest) to the stream listener on the cache entry. This causes lots of
stuff to fail.

In addition to the nsIChannel problem, we also have fall out because
nsMemCacheTransport->mCurrentReadRequest never gets initialized so
AsyncReadStreamAdaptor drops out with errors all over the place because
mCurrentReadRequest is null.

Comment 5

[gordon]

Adding darin and rpotts, since they reviewed dougt's checkin, and gagan, just for 
kicks.

Comment 6

[Scott MacGregor]

Changing summary to: memory cache defunct after landing =).

QA: one of the symptoms to verify for this bug is that you were unable to open
mail attachment or reply to messages.

Comment 7

[Bradley Baetz]

Oops - accidentally deleted my comment.

my gopher stuff has the same problem (nsGopherDirListingConv::OnDataAvailable
uses the request to QI for a channel to get the URI, which now fails, and hangs
the transfer). The interesting bit is that the FTPDirListing converter does
exactly the same thing, except that it works. From what I can see,
nsFtpConnectionThread uses a DataRequestForwarder as a proxy to get arround
this.

Comment 8

[Doug Turner (:dougt)]

The DataRequestForwarder makes sure that the client does not get the raw ftp 
data socket.  Maybe gopher needs to do something similar.

It has nothing to do with the memcache being really broken (afaict)

Comment 9

[Doug Turner (:dougt)]

*** Bug 68371 has been marked as a duplicate of this bug. ***

Comment 10

[Doug Turner (:dougt)]

The cache use to support nsIChannel interfaces.  Now it supports nsITransport's. 
 The problem is that content information has been dropped.  

A solution is to have our cache'ed objects represent both.  

So, when I say |newTransport|  I get an object back that implements both a 
transport and a channel.

Comment 11

[Doug Turner (:dougt)]

file cache suffers from the same problem.  Adjusting summary.

Comment 12

[Doug Turner (:dougt)]

spoke with mscott last night and there is a workaround for IMAP in place.  I 
don't think that this should be a blocker.  

Comment 13

[lchiang]

What is the workaround referenced above?  In the code or for the user to do? 
Thanks.

Comment 14

[Scott MacGregor]

Lisa, sorry for not being clear. I checked in  work around late Friday night.
Users won't have to do anything. performance will just suffer a little bit since
we won't be leveraging the memory cache.

Comment 15

[Darin Fisher]

These things worked on the branch but not on the trunk, right?  Shouldn't we
be looking for differences between the two then?

Comment 16

[Darin Fisher]

Ok, I've made the cache "channels" support both nsIChannel and nsITransport
to get around this problem.

The fix is on dougt's branch DOUGT_CHANNEL_CHANGES_02112001_BRANCH.

Comment 17

[Doug Turner (:dougt)]

This bug should be fixed my recently checked in necko changes.  

Comment 18

[esther]

Using builds dated 03-01-01 on mac and linux Reply and opening an attachment 
(.jpg)work. Also working on build dated 03-01--2 on winme.  Will verify that 
these work now, but if this is a temporary or work around fix, then reopen this 
or log a new one for addtional fixing.