fair-acc / opencmw-cpp

Open Common Middle-Ware library for accelerator equipment- and beam-based control systems at FAIR.
https://opencmw.io
GNU Lesser General Public License v3.0
10 stars 8 forks source link

Do not block worker threads forever if no event is received (#329) #330

Closed frankosterfeld closed 12 months ago

frankosterfeld commented 1 year ago

Make sure that long-poll request handlers do not block forever when no corresponding event is received. Otherwise the clients will send one request after another once their request times out client-side, until the worker threads are exhausted and the server stops responding.

It would be great if we could also detect the client connection be dropped using Keep-Alive, but cpp-httplib doesn't seem to have API for that.

codecov[bot] commented 12 months ago

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (c19531d) 55.75% compared to head (260e752) 55.80%.

:exclamation: Current head 260e752 differs from pull request most recent head 6444125. Consider uploading reports for the commit 6444125 to get more accurate results

Files Patch % Lines
src/majordomo/include/majordomo/RestBackend.hpp 14.28% 12 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #330 +/- ## ========================================== + Coverage 55.75% 55.80% +0.04% ========================================== Files 69 69 Lines 7291 7299 +8 Branches 2691 2693 +2 ========================================== + Hits 4065 4073 +8 - Misses 1489 1490 +1 + Partials 1737 1736 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 12 months ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

4.9% 4.9% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

frankosterfeld commented 12 months ago

This seems to look OK but should be accompanied by a unit-tests that demonstrate the feature/workaround.

I've made the timeout configurable and also applied it to GET/SET now. I had a hard time to create a working subscription test case, so I've rebased this now against the changes in #331 and made in part of #331 (has both changes now).