Last Comment Bug 670277 - WARNING: NS_ENSURE_TRUE(mState == STATE_TRANSFERRING) failed: file netwerk/base/src/nsSocketTransport2.cpp
: WARNING: NS_ENSURE_TRUE(mState == STATE_TRANSFERRING) failed: file netwerk/ba...
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla12
Assigned To: Patrick McManus [:mcmanus]
: Patrick McManus [:mcmanus]
Depends on:
  Show dependency treegraph
Reported: 2011-07-08 16:26 PDT by Daniel Holbert [:dholbert] (vacation, returning 2/27)
Modified: 2012-01-27 09:09 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v0 (3.77 KB, patch)
2012-01-26 11:05 PST, Patrick McManus [:mcmanus]
cbiesinger: review+
Details | Diff | Splinter Review

Description User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 2011-07-08 16:26:11 PDT
 1. Load any remote image in a debug build, e.g.:
 2. Hold down Ctrl + R (to perform many repeated reloads)

This warning gets printed in my terminal:
WARNING: NS_ENSURE_TRUE(mState == STATE_TRANSFERRING) failed: file ../../../../mozilla/netwerk/base/src/nsSocketTransport2.cpp, line 1889

No warning.  This is a perfectly normal use-case (interrupting an in-progress load with a reload), and normal use-cases shouldn't trigger warning-spam.

(This same warning gets printed for lots of other sorts of reloads, too - e.g. if I load and reload while it's loading.  It's simplest with a single image, though, because there's less other noise going by in the terminal.)
Comment 1 User image Honza Bambas (:mayhemer) 2011-08-03 08:36:09 PDT
I was able to reproduce only with mState == STATE_CLOSED (== 0).  We might extend the check to also include STATE_CLOSED but only while mNetAddr has a valid (or simply not invalid) family set.
Comment 2 User image Honza Bambas (:mayhemer) 2011-08-03 08:40:04 PDT
Or, when state is not STATE_RESOLVING and mNetAddr is not invalid.  Because only during the RESOLVING phase the structure may change.
Comment 3 User image Patrick McManus [:mcmanus] 2012-01-26 10:56:37 PST
The common trigger for this seems to be an asynchronous nsITransportStatusEvent that calls GetPeerAddr() after the connection has closed.

The code should not be using NS_ENSURE_TRUE() which produces the WARNING - calling GetPeerAddr on an unconnected socket will return an error but it isn't a violation to make the call. I will change that to be a normal conditional plus a LOG message.

I will also enhance this to match the idl comment which says GetPeerAddr() is valid once the connection has been established (which includes after it is closed) by setting a flag when we get to TRANSFERRING and allowing access to mNetAddr anytime after that point.

patch to follow.
Comment 4 User image Patrick McManus [:mcmanus] 2012-01-26 11:05:59 PST
Created attachment 591863 [details] [diff] [review]
patch v0
Comment 5 User image Christian :Biesinger (don't email me, ping me on IRC) 2012-01-26 11:09:23 PST
Comment on attachment 591863 [details] [diff] [review]
patch v0

     SOCKET_LOG(("  advancing to STATE_TRANSFERRING\n"));
     mPollTimeout = mTimeouts[TIMEOUT_READ_WRITE];
+    mNetAddrIsSet = true;

Please add a comment here about why you set this to true only now, i.e. that this is the earliest time where we can be sure what host we're establishing the connection to due to failover.

sidenote, this is why I don't like the NS_ENSURE_* macros
Comment 6 User image Patrick McManus [:mcmanus] 2012-01-26 13:12:20 PST
Comment 7 User image Matt Brubeck (:mbrubeck) 2012-01-27 09:09:26 PST

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