Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't terminate the server when socket operation fails #996

Merged
merged 5 commits into from
Jun 7, 2023

Conversation

joka921
Copy link
Member

@joka921 joka921 commented Jun 1, 2023

No description provided.

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Patch coverage: 94.73% and no project coverage change.

Comparison is base (7920f8e) 74.07% compared to head (72bb26a) 74.07%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #996   +/-   ##
=======================================
  Coverage   74.07%   74.07%           
=======================================
  Files         254      254           
  Lines       23991    23997    +6     
  Branches     3020     3021    +1     
=======================================
+ Hits        17771    17776    +5     
  Misses       5010     5010           
- Partials     1210     1211    +1     
Impacted Files Coverage Δ
.../util/OnDestructionDontThrowDuringStackUnwinding.h 91.66% <75.00%> (-3.34%) ⬇️
src/parser/sparqlParser/SparqlQleverVisitor.cpp 87.55% <100.00%> (+0.01%) ⬆️
src/util/Synchronized.h 96.87% <100.00%> (ø)
src/util/http/HttpServer.h 72.54% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hannahbast hannahbast changed the title Don't terminate the Server when closing the socket fails. Don't terminate the server when socket operation fails Jun 2, 2023
Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this fix, here is a suggestion for the text for the commit message (for whowever merges the PR in the end):

When a socket operation was aborted, the closing of the socket threw an exception due to a subtle bug. That exception was then not caught and the server was terminated. The bug is now fixed and all exceptions are now caught by handling the cleanup with our own ad_utility::makeOnDestructionDontThrowDuringStackUnwinding instead of with absl::Cleanup.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@hannahbast hannahbast merged commit db76022 into ad-freiburg:master Jun 7, 2023
@joka921 joka921 deleted the catch_boost_exceptions branch July 12, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants