Last Comment Bug 412311 - Replace PR_INTERVAL_NO_WAIT with PR_INTERVAL_NO_TIMEOUT in client initialization calls
: Replace PR_INTERVAL_NO_WAIT with PR_INTERVAL_NO_TIMEOUT in client initializat...
Status: RESOLVED FIXED
PKIX SUN_MUST_HAVE
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
: P2 normal (vote)
: 3.12.2
Assigned To: Alexei Volkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-14 10:29 PST by Alexei Volkov
Modified: 2008-10-24 20:57 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 use global pkix timeout (9.66 KB, patch)
2008-10-14 16:08 PDT, Alexei Volkov
nelson: review-
Details | Diff | Splinter Review
Patch v2 - use nsscontext stored timeout (34.21 KB, patch)
2008-10-20 16:16 PDT, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review
Patch v3 - integrated (41.91 KB, patch)
2008-10-24 15:59 PDT, Alexei Volkov
no flags Details | Diff | Splinter Review

Description Alexei Volkov 2008-01-14 10:29:01 PST
PR_INTERVAL_NO_WAIT socket option makes client to use non-blocking IO. It is still untested(and apparently broken) feature, and so it should not be used until verified(or fixed).
Comment 1 Nelson Bolyard (seldom reads bugmail) 2008-01-14 11:43:30 PST
Alexei, this bug is WAY too vague.  It sounds like you're saying that 
non-blocking IO is not supported anywhere by NSS.  But in fact, 
non-blocking IO has been working for years in libSSL.  

I suspect you are referring to some specific function, or small group
of functions, but I don't know in what libraries or test programs.
So, please correct this bug to be very specific about the issue.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2008-01-14 11:48:18 PST
After reading bug 408434 comment 8, I gather this bug is describing 
function pkix_pl_AiaMgr_FindLDAPClient in 
nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_aiamgr.c .
Is there any other function that is covered by this bug?
Comment 3 Alexei Volkov 2008-01-16 15:16:17 PST
> Alexei, this bug is WAY too vague.  It sounds like you're saying that 
> non-blocking IO is not supported anywhere by NSS.  But in fact, 
> non-blocking IO has been working for years in libSSL.  

This bug speaks only about libpkix non-blocking functionality.

> Is there any other function that is covered by this bug?
No. Just this one place. Bug any IO feature of libpkix(cert, crl fetching, ocsp revocation checking) can, by design, use NBIO. We just need to make sure that libpkix does not use NBIO before this libpkix feature get tested.  
Comment 4 Nelson Bolyard (seldom reads bugmail) 2008-06-06 15:56:25 PDT
We need to think more about how timeouts are set.
Comment 5 Alexei Volkov 2008-06-11 15:34:25 PDT
Retargeting to 3.12.1. Want to make sure timeouts are properly set.
Comment 6 Alexei Volkov 2008-10-14 16:08:59 PDT
Created attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

The set of changes: create global pkix structure that keeps values for pkix communication timeout and maximum transfer length. Use those values for ocsp 
responses and cert/crl transfers.
Comment 7 Alexei Volkov 2008-10-14 16:11:41 PDT
Comment on attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

The intention of the setter function is to change default values, but those can only use before/immediately after pkix initialization.
Comment 8 Nelson Bolyard (seldom reads bugmail) 2008-10-15 14:32:52 PDT
Comment on attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

Let's not define a new global, except as a _DEFAULT_ value.
Let's put the actual values used the PL Context struct.
CERT_PKIXVerifyCert should have a way for the caller to 
specify values, and if he doesn't then the defaults should 
be used.  It's OK for the defaults to be settable, but 
setting the defaults should not be the only way to change 
these values.
Comment 9 Alexei Volkov 2008-10-20 16:16:35 PDT
Created attachment 343984 [details] [diff] [review]
Patch v2 - use nsscontext stored timeout

The patch also cleans up some function in pkix_pl_aiamgr.c and fixes http context leak in pkix_pl_httpcertstore.c.
Comment 10 Nelson Bolyard (seldom reads bugmail) 2008-10-21 16:08:38 PDT
Comment on attachment 343984 [details] [diff] [review]
Patch v2 - use nsscontext stored timeout

r+

I think maybe the setter functions below belong in a public header?

>--- lib/libpkix/pkix_pl_nss/module/pkix_pl_nsscontext.h	31 Aug 2007 03:21:38 -0000	1.3
>+++ lib/libpkix/pkix_pl_nss/module/pkix_pl_nsscontext.h	20 Oct 2008 22:42:10 -0000
>@@ -73,6 +73,12 @@ pkix_pl_NssContext_GetWincx(PKIX_PL_NssC
> PKIX_Error *
> pkix_pl_NssContext_SetWincx(void *wincx, PKIX_PL_NssContext *nssContext);
> 
>+PKIX_Error *
>+pkix_pl_NssContext_SetTimeout(PKIX_UInt32 timeout, PKIX_PL_NssContext *nssContext);
>+
>+PKIX_Error *
>+pkix_pl_NssContext_SetMaxResponseLen(PKIX_UInt32 len, PKIX_PL_NssContext *nssContext);
>+
Comment 11 Alexei Volkov 2008-10-24 15:59:21 PDT
Created attachment 344694 [details] [diff] [review]
Patch v3 - integrated

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