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

Promote PendingAction for general use. #8423

Merged

Conversation

SolidWallOfCode
Copy link
Member

PendingAction is a class used in HttpSM to safely tracking pending actions. This is generally useful. In particular there is a race condition in PluginVC which should be fixed by using this class.

In addition to promoting this out of "HttpSM.h" for general use, the implementation has been adjusted for greater thread safety. Multiple threads can safely assign to the instance, with the guarantee that all actions except the one stored in the instance have been canceled.

@SolidWallOfCode SolidWallOfCode added this to the 10.0.0 milestone Oct 14, 2021
@SolidWallOfCode SolidWallOfCode self-assigned this Oct 14, 2021
Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Looks fine.

proxy/http/HttpSM.cc Show resolved Hide resolved
Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

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

Make sense.

@maskit
Copy link
Member

maskit commented Oct 15, 2021

In addition to promoting this out of "HttpSM.h" for general use, the implementation has been adjusted for greater thread safety. Multiple threads can safely assign to the instance, with the guarantee that all actions except the one stored in the instance have been canceled.

I don't like that this is done at the same time with the promotion. It's not even a separate commit. It's not reviewer friendly because you can't simply see the additional changes, and we can't revert it separately if it has problems.

@SolidWallOfCode
Copy link
Member Author

Updated to remove the thread safety, I'll put that in a separate PR.

@SolidWallOfCode SolidWallOfCode merged commit c72d433 into apache:master Oct 20, 2021
@bneradt
Copy link
Contributor

bneradt commented May 12, 2022

#8846 adds #pragma once for this new header, which will be needed as this is used more. This is currently only marked for 10.x, but if we wind up bringing this back to 9.x we'll want to bring #8846 with it.

@bneradt
Copy link
Contributor

bneradt commented May 12, 2022

#8846 adds #pragma once for this new header, which will be needed as this is used more. This is currently only marked for 10.x, but if we wind up bringing this back to 9.x we'll want to bring #8846 with it.

Actually, I'd like to get this into 9.2.x since #8847 uses PendingAction. Marking for 9.2.x.

@zwoop
Copy link
Contributor

zwoop commented May 16, 2022

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 May 16, 2022
zwoop pushed a commit that referenced this pull request May 16, 2022
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
* asf/9.2.x:
  Updated ChangeLog
  Fix parent_select optional scheme (apache#8831)
  Add `#pragma once` for PendingAction.h (apache#8846)
  Promote class PendingAction from HttpSM.h for use in other classes. (apache#8423)
  Fixes leak in SNIAction name globbing (apache#8827)
  Handle opentelemetry-cpp v1.3.0 upgrade for otel_tracer plugin (apache#8834)
  Fix overflow conditions in prefetch plugin (apache#8660)
  Remove incorrect comment from base64 functions (apache#8835)
  Add autest to cover updates to cache with alternates (apache#8779)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants