Closed Bug 143172 Opened 20 years ago Closed 20 years ago

set low connect timeout for LDAP XPCOM SDK by default

Categories

(Directory :: LDAP XPCOM SDK, defect, P1)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: dmose, Assigned: dmose)

Details

(Keywords: hang, Whiteboard: [adt2] [fixed on trunk] [fixed on mozilla1.0.0 branch])

Attachments

(1 file, 1 obsolete file)

This is a very simple fix pointed out by mcs that makes bug 79509 much less
painful.  Users will perceive it as a hiccup or stutter by the browser suite,
not a hang that forces them to kill the entire thing.
Status: NEW → ASSIGNED
Keywords: mozilla1.0+, nsbeta1
Priority: -- → P1
Target Milestone: --- → mozilla1.0
In bug 79509, mcs said:

> Also, note that you should be able to reduce the connect timeout when you are
> NOT using the LDAP_OPT_ASYNC_CONNECT option by making a call like this:
>
> int ms = 10000; /* 10 seconds, in milliseconds */
> if ( ldap_set_option( 0, LDAP_X_OPT_CONNECT_TIMEOUT, *ms ) != 0 ) {
>      /* error */
> }

[...]

> IMHO a timeout of 10 seconds seems more than long enough for autocomplete (of 
> course it should be configurable).




I decided to use the prldap mechanism instead, as this will save us from
infinite hangs on all operations; I also chose a slightly lower timeout of 5
seconds.  Yeah, this should be a preference, and I'll file a bug for that, but
I'd like to get this into 1.0, so I'm trying to keep the patch small.
As far as the old XXX comment, I think that was just because I was setting the
timeout to 0, rather than to something large enough to succeed most of the time.

Reviews welcomed.  :-)
Comment on attachment 82881 [details] [diff] [review]
set an upper limit of 5 seconds on all I/O operations

sr=sspitzer
Attachment #82881 - Flags: superreview+
Dan, how confident are you about what you wrote in comment #3 about your code
comment not being something we should be worried about?

Let's get a r= for this.  I'll bring this to adt.
Keywords: nsbeta1hang, nsbeta1+
Whiteboard: [adt2]
Comment on attachment 82881 [details] [diff] [review]
set an upper limit of 5 seconds on all I/O operations

r=mcs, but I am a little worried that 5 seconds will be too short for the
initial TCP connect timeout (it really depends on the network between your
machine and the LDAP server). But the default TCP connect timeout is 1-2
minutes or so on most platforms. I still vote for 10 seconds, but can live with
5.
Attachment #82881 - Flags: review+
Comment on attachment 82881 [details] [diff] [review]
set an upper limit of 5 seconds on all I/O operations

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #82881 - Flags: approval+
If it would be safer to do 10 seconds then let's do that.  It's still way better
than a couple of minutes.

Could we get this in a test build for QA?  If we need to respin and it's been
verified, this would be worth considering for beta.
putterman: I'd say I'm about 75% confident of comment number 3.  I'll generate a
new patch for switching to 10 seconds and checkin on the trunk.  So trunk builds
for tommorrow should have this patch, and QA can test against those.
Bump upper limit to 10 seconds.
Attachment #82881 - Attachment is obsolete: true
Comment on attachment 82980 [details] [diff] [review]
set an upper limit of 10 seconds on all I/O operations

Carrying forward review, super-review, approval.
Attachment #82980 - Flags: superreview+
Attachment #82980 - Flags: review+
Attachment #82980 - Flags: approval+
QA: a good way to forcibly provoke the hang for testing purposes is to set up a
server pointing at IP address 1.1.1.1 port 389, and try an LDAP operation
against that.
I just tried out dmose's patch on my trunk, mozilla, win2k build.

It worked like a charm.

Fix checked in on the trunk.
Whiteboard: [adt2] → [adt2] [fixed on trunk]
adding adt1.0.0+ for checkin to mozilla1.0 branch.
Keywords: adt1.0.0+
Whiteboard: [adt2] [fixed on trunk] → [adt2] [fixed on trunk] [fixed on mozilla1.0.0 branch]
20020510 trunk build on MacOS 9.1 and Linux

Works fine.
This won't make the Netscape beta train, so resolving as fixed.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED
Verified with 20020605 branch build on Win2K and MacOS 9.2.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.