fix: WaitOnSocket select error when sockfd above FD_SETSIZE

[taozhijiang]

Fixes select return code is not well handled, and when curl sockfd above FD_SETSIZE (1024), select can not handle it.

Changes:

1. According docs, select return -1 on error, 0 when timeout, and >0 for expected events return. The original cast the int code to bool, so when invalid sock provided, the procedure treat as good, the following procedure crash. So the code carefully check the select return code, and the really events we get.
2. Even though select is POSIX standard and cross platform, but can not handle the case when fd above FD_SETSIZE. In our production environment, we see CURL sock fd far more above it, so when Linux environment possible, we use poll instead of select.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: taozhijiang / name: ztao (3bbaaa1)

[codecov]

Codecov Report

Merging #1410 (e02c2a0) into main (63803d1) will decrease coverage by 0.03%.
The diff coverage is 81.82%.

@@            Coverage Diff             @@
##             main    #1410      +/-   ##
==========================================
- Coverage   85.02%   84.99%   -0.02%     
==========================================
  Files         152      152              
  Lines        4350     4350              
==========================================
- Hits         3698     3697       -1     
- Misses        652      653       +1     

Impacted Files	Coverage Δ
...lemetry/ext/http/client/curl/http_operation_curl.h	77.64% <81.82%> (-0.65%)	⬇️

[owent]

Could you please do this change base on async-changes branch? The select call wil be removed after we merge this branch back into main.

[lalitb]

Could you please do this change base on async-changes branch? The select call wil be removed after we merge this branch back into main.

@owent - Are these changes relevant once async-changes branch is merged to main? If not, we can let these changes merged in the main branch for now, and later these changes would be removed once async-changes branch is merged to main. Would leave it to you to decide :)

[taozhijiang]

Could you please do this change base on async-changes branch? The select call wil be removed after we merge this branch back into main.

@owent - Are these changes relevant once async-changes branch is merged to main? If not, we can let these changes merged in main branch for now, and later these changes would be removed once async-changes branch is merged to main. Would leave it to you to decide :)

This is a definite code defect which may affect the delopyment in production, so I think should be fixed ASAP.
We will try async-changes latter in test environment when we are available ;-)

[owent]

relevant

Yes, the implement in async-changes branch will use curl_multi_poll to replace select. This problem may still exist because without event loop system, curl_multi_poll use select on some platform, but we will reuse connections for multiple sessions now, and when max_concurrent_requests / max_requests_per_connection is less than 1023, we will have 1024 fds at most at the same time.The default value of max_concurrent_requests is 64 and default value max_requests_per_connection is 8 now, which means will have no more than 9 connections at most by default.

I will continue work on merge async-changes back to main on these days.

[owent]

Could you please do this change base on async-changes branch? The select call wil be removed after we merge this branch back into main.

@owent - Are these changes relevant once async-changes branch is merged to main? If not, we can let these changes merged in main branch for now, and later these changes would be removed once async-changes branch is merged to main. Would leave it to you to decide :)

This is a definite code defect which may affect the delopyment in production, so I think should be fixed ASAP. We will try async-changes latter in test environment when we are available ;-)

Sure, if you don't mind these changes in this PR may be removed later.
This PR also fixes #1220 .

[owent]

LGTM after the format is fixed.

[lalitb]

@taozhijiang - You can fix the format as documented here - https://github.com/open-telemetry/opentelemetry-cpp/blob/main/CONTRIBUTING.md#pull-requests

[lalitb]

LGTM after the format is fixed.

@owent - Can you remove the requested change if it looks good to merge. Thanks.

[owent]

LGTM after the format is fixed.

@owent - Can you remove the requested change if it looks good to merge. Thanks.

Done