Bug 11564 - async_req: fix non-blocking connect()
Summary: async_req: fix non-blocking connect()
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-20 14:59 UTC by Ralph Böhme
Modified: 2015-10-29 08:40 UTC (History)
1 user (show)

See Also:


Attachments
Patch for master (9.05 KB, patch)
2015-10-20 15:19 UTC, Ralph Böhme
no flags Details
Patch for master (8.79 KB, patch)
2015-10-20 15:45 UTC, Ralph Böhme
no flags Details
Fixed version - port should be uint16_t. (8.92 KB, patch)
2015-10-20 17:49 UTC, Jeremy Allison
no flags Details
git-am fix for 4.3.next. (9.06 KB, patch)
2015-10-20 22:01 UTC, Jeremy Allison
slow: review-
Details
git-am fix for 4.2.next. (3.75 KB, patch)
2015-10-20 22:01 UTC, Jeremy Allison
slow: review-
Details
git-am fix for 4.3.next. (9.21 KB, patch)
2015-10-21 22:56 UTC, Jeremy Allison
slow: review+
Details
git-am fix for 4.2.next. (9.20 KB, patch)
2015-10-21 22:57 UTC, Jeremy Allison
slow: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2015-10-20 14:59:00 UTC
According to Stevens UNIX Network Programming and various other sources,
the correct handling for non-blocking connect() is:

- when the initial connect() return -1/EINPROGRESS polling the socket
  for *writeability*

- in the poll handler call getsocktopt() with SO_ERROR to get the
  finished connect() return value

Simply calling connect() a second time without error checking is
probably wrong and not portable. For a successfull connect() Linux
returns 0, but Solaris will return EISCONN:

24254:   0.0336  0.0002 connect(4, 0xFEFFECAC, 16, SOV_DEFAULT) Err#150 EINPROGRESS
24254:          AF_INET  name = 10.10.10.143  port = 1024
24254:   0.0349  0.0001 port_associate(3, 4, 0x00000004, 0x0000001D,0x080648A8) = 0
24254:   0.0495  0.0146 port_getn(3, 0xFEFFEB50, 1, 1, 0xFEFFEB60) = 1 [0]
24254:   0.0497  0.0002 connect(4, 0x080646E4, 16, SOV_DEFAULT) Err#133 EISCONN
24254:          AF_INET  name = 10.10.10.143  port = 1024

Patch to folllow.
Comment 1 Ralph Böhme 2015-10-20 15:19:00 UTC
Created attachment 11511 [details]
Patch for master
Comment 2 Ralph Böhme 2015-10-20 15:45:17 UTC
Created attachment 11512 [details]
Patch for master
Comment 3 Jeremy Allison 2015-10-20 17:49:59 UTC
Created attachment 11513 [details]
Fixed version - port should be uint16_t.
Comment 4 Jeremy Allison 2015-10-20 22:01:10 UTC
Created attachment 11515 [details]
git-am fix for 4.3.next.

Cherry-picked from master.
Comment 5 Jeremy Allison 2015-10-20 22:01:56 UTC
Created attachment 11516 [details]
git-am fix for 4.2.next.

Cherry-picked from master (minus test, which doesn't apply cleanly - not needed for code fix anyway).
Comment 6 Ralph Böhme 2015-10-21 07:02:15 UTC
Comment on attachment 11516 [details]
git-am fix for 4.2.next.

We need the fix for the memset() parameter bug Andrew discovered, either squashed or appended. Will follow-up with an updated patchset for 4.2 and 4.3.
Comment 7 Ralph Böhme 2015-10-21 07:02:25 UTC
Comment on attachment 11515 [details]
git-am fix for 4.3.next.

We need the fix for the memset() parameter bug Andrew discovered, either squashed or appended. Will follow-up with an updated patchset for 4.2 and 4.3.
Comment 8 Jeremy Allison 2015-10-21 16:44:42 UTC
Sorry for missing the memset problem - compiled without warnings so I didn't spot it.

Cheers,

Jeremy.
Comment 9 Jeremy Allison 2015-10-21 22:56:33 UTC
Created attachment 11521 [details]
git-am fix for 4.3.next.

Cherry-picked from master + squashed memset fix.
Comment 10 Jeremy Allison 2015-10-21 22:57:11 UTC
Created attachment 11522 [details]
git-am fix for 4.2.next.

Contains squash of memset fix.
Comment 11 Ralph Böhme 2015-10-22 07:04:28 UTC
Reassigning to Karolin for inclusion in 4.2 and 4.3. Thanks!
Comment 12 Karolin Seeger 2015-10-28 09:53:08 UTC
(In reply to Ralph Böhme from comment #11)
Pushed to autobuild-v4[3|2]-test.
Comment 13 Karolin Seeger 2015-10-29 08:40:23 UTC
Pushed to v4-2-test and v4-3-test.
Closing out bug report.

Thanks!