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.

Sign up for GitHub

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

Jump to bottom

Metalink Plugin: Must not destroy the Transform Contp, only shutdown write on it while received WRITE_COMPLETE #1540

Merged
merged 1 commit into from Aug 16, 2017
Merged

Metalink Plugin: Must not destroy the Transform Contp, only shutdown write on it while received WRITE_COMPLETE #1540

merged 1 commit into from Aug 16, 2017

Conversation

oknet
Copy link
Member

@oknet oknet commented Mar 6, 2017 •
edited

The Transform Contp is belongs to TransformVC chain and it will be closed by TransformVC. And then the plugin will be callback with EVENT_IMMEDIATE, the plugin will find the Transform Contp is closed by TSVConnClosedGet() and call TSContDestroy() to destroy it.

@oknet oknet changed the title Must not destroy the Transform Contp, only shutdown write on it while received WRITE_COMPLETE Metalink Plugin: Must not destroy the Transform Contp, only shutdown write on it while received WRITE_COMPLETE Mar 6, 2017
@oknet oknet self-assigned this Mar 6, 2017
@oknet
Copy link
Member Author

oknet commented Mar 6, 2017

@jablko

@zwoop
Copy link
Contributor

zwoop commented Mar 6, 2017

@oknet Can you please add the appropriate Label andMilestone for this PR? ("Plugins" and "7.2.0" respectively).

@zwoop
Copy link
Contributor

zwoop commented Mar 7, 2017

I think this would be good for @jablko to review. Also, Jack, I think you have to go through the MATT process again to link your account, I'm not able to assign this to you for review.

@oknet oknet added the Plugins label Mar 7, 2017
@oknet oknet added this to the 7.2.0 milestone Mar 7, 2017
@oknet
Copy link
Member Author

oknet commented Mar 7, 2017

@zwoop Could you create a new Label：Transform ？

@zwoop
Copy link
Contributor

zwoop commented Mar 7, 2017

[approve ci] to test the new Github builds jobs configs.

@oknet
Metalink Plugin: Must not destroy the Transform Contp, only shutdown …
b75d07d 
…write on it while received WRITE_COMPLETE

The transform contp will be closed and destroied after TransformVC is closed.
The transformation plugin receives EVENT_IMMEDIATE when transform contp close
then the plugin could release 'TransformData' and call TSContDestroy().
@atsci
Copy link

atsci commented Mar 23, 2017

clang format successful! https://ci.trafficserver.apache.org/job/clang-format-github/93/

@atsci
Copy link

atsci commented Mar 23, 2017

RAT check successful! https://ci.trafficserver.apache.org/job/RAT-github/106/

@atsci
Copy link

atsci commented Mar 23, 2017

AU check failed! https://ci.trafficserver.apache.org/job/autest-github/90/

@atsci
Copy link

atsci commented Mar 23, 2017

FreeBSD11 build successful! https://ci.trafficserver.apache.org/job/freebsd-github/1788/

@atsci
Copy link

atsci commented Mar 23, 2017

Linux build successful! https://ci.trafficserver.apache.org/job/linux-github/1681/

@atsci
Copy link

atsci commented Mar 23, 2017

Intel CC build successful! https://ci.trafficserver.apache.org/job/icc-github/219/

@atsci
Copy link

atsci commented Mar 23, 2017

clang-analyzer build successful! https://ci.trafficserver.apache.org/job/clang-analyzer-github/351/

@oknet
Copy link
Member Author

oknet commented Mar 23, 2017

The transform contp will be closed and destroied after TransformVC is closed.
The transformation plugin receives EVENT_IMMEDIATE when transform contp close then the plugin could release 'TransformData' and call TSContDestroy().

@zwoop
Copy link
Contributor

zwoop commented Mar 27, 2017

@jablko Can you review this please? And also, I think your MATT connection needs to be redone, since we got moved to gitbox for the internal ASF sync service).

@zwoop zwoop modified the milestones: 7.2.0, 8.0.0 Apr 25, 2017
@oknet
Copy link
Member Author

oknet commented May 20, 2017

[approve ci]

@zwoop
Copy link
Contributor

zwoop commented Jun 8, 2017

Should we merge this? We might have lost @jablko ?? :)

@zwoop zwoop added the Transform label Jun 8, 2017
zwoop
zwoop approved these changes Jul 20, 2017
View reviewed changes
Copy link
Contributor

@zwoop zwoop left a comment

Choose a reason for hiding this comment

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

I'm +1 on this, with the caveat that I have not tested it (I assume you did). Can @scw00 take a look as well ?

@zwoop zwoop merged commit e115703 into apache:master Aug 16, 2017
@zwoop
Copy link
Contributor

zwoop commented Aug 16, 2017

If this is a candidate for 7.x, please mark it as such (in the Projects tab).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zwoop zwoop zwoop approved these changes

@SolidWallOfCode SolidWallOfCode Awaiting requested review from SolidWallOfCode

Assignees

@oknet oknet

Labels
Plugins Transform
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@oknet @zwoop @atsci