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...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: Patrick McManus [:mcmanus]
:
: Patrick McManus [:mcmanus]
Mentors:
https://bugzilla.mozilla.org/skins/st...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-08 16:26 PDT by Daniel Holbert [:dholbert]
Modified: 2012-01-27 09:09 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description Daniel Holbert [:dholbert] 2011-07-08 16:26:11 PDT
STEPS TO REPRODUCE:
 1. Load any remote image in a debug build, e.g.:
     https://bugzilla.mozilla.org/skins/standard/index/help.png
 2. Hold down Ctrl + R (to perform many repeated reloads)

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

EXPECTED RESULTS:
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 mozilla.org 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 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 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 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 Patrick McManus [:mcmanus] 2012-01-26 11:05:59 PST
Created attachment 591863 [details] [diff] [review]
patch v0
Comment 5 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

 nsSocketTransport::OnSocketConnected()
 {
     SOCKET_LOG(("  advancing to STATE_TRANSFERRING\n"));
 
     mPollFlags = (PR_POLL_READ | PR_POLL_WRITE | PR_POLL_EXCEPT);
     mPollTimeout = mTimeouts[TIMEOUT_READ_WRITE];
     mState = STATE_TRANSFERRING;
+    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 Patrick McManus [:mcmanus] 2012-01-26 13:12:20 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4b103adf7fc
Comment 7 Matt Brubeck (:mbrubeck) 2012-01-27 09:09:26 PST
https://hg.mozilla.org/mozilla-central/rev/c4b103adf7fc

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