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

plugin: fix audit plugin will cause tidb panic #23803

Merged
merged 12 commits into from
Apr 2, 2021
Merged

plugin: fix audit plugin will cause tidb panic #23803

merged 12 commits into from
Apr 2, 2021

Conversation

AilinKid
Copy link
Contributor

@AilinKid AilinKid commented Apr 1, 2021 •
edited
Loading

Signed-off-by: AilinKid 314806019@qq.com

What problem does this PR solve?

Issue Number: close #23786

What is changed and how it works?

What's Changed: fix the sessionVars.connectionInfo may be nil when exit a conn.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Manual test (add detailed scripts or steps below)
1: tiup playground nightly --tiflash 0  
2: substitute the tidb bin with the ours
3: open a leading conn1 and set the audit plugin as false ( admin plugins disable audit;)
5: open a test conn2
6: enable the audit plugin in the leading con1 ( admin plugins enable audit;)
7: exit the test conn2
before this pr: tidb panic

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x2f5de34]
goroutine 809 [running]:
github.com/pingcap/tidb/server.(*Server).onConn.func4(0xc0006d3590, 0xc0006d3380, 0xc00155ddee)
/home/arenatlx/go/src/github.com/pingcap/tidb/server/server.go:486 +0xb4
github.com/pingcap/tidb/plugin.ForeachPlugin(0xc0011e6d01, 0xc00155df00, 0xc00141ba10, 0x0)
/home/arenatlx/go/src/github.com/pingcap/tidb/plugin/plugin.go:419 +0x137
github.com/pingcap/tidb/server.(*Server).onConn(0xc0012c2dd0, 0xc0011e6d00)
/home/arenatlx/go/src/github.com/pingcap/tidb/server/server.go:482 +0xaf9
created by github.com/pingcap/tidb/server.(*Server).Run
/home/arenatlx/go/src/github.com/pingcap/tidb/server/server.go:383 +0x8d5

this pr

HAPPY

Release note

  • plugin: fix audit plugin will cause tidb panic

@AilinKid
fix audit plugin will cause tidb panic
c6b6c7f 
Signed-off-by: AilinKid <314806019@qq.com>
@ti-chi-bot ti-chi-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 1, 2021
@AilinKid AilinKid requested review from wshwsh12 and lysu April 1, 2021 08:24
@AilinKid AilinKid added type/bugfix This PR fixes a bug. sig/sql-infra SIG: SQL Infra needs-cherry-pick-4.0 labels Apr 1, 2021
@lysu
Copy link
Contributor

lysu commented Apr 1, 2021

/lgtm

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 1, 2021
@wshwsh12
Copy link
Contributor

wshwsh12 commented Apr 1, 2021

/lgtm

@ti-chi-bot
Copy link
Member

@wshwsh12: /lgtm is only allowed for the reviewers in list.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

djshow832
djshow832 reviewed Apr 1, 2021
View reviewed changes
Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

Add some test cases

@AilinKid @djshow832
Update server/server.go
003448c 
Co-authored-by: djshow832 <zhangming@pingcap.com>
@djshow832
Copy link
Contributor

/lgtm

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • djshow832
  • lysu

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 1, 2021
@djshow832
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 003448c

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 1, 2021
@AilinKid
Copy link
Contributor Author

AilinKid commented Apr 1, 2021

/merge

@ti-chi-bot ti-chi-bot merged commit 5f8ec4b into pingcap:master Apr 2, 2021
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Apr 2, 2021
@AilinKid @ti-srebot
cherry pick pingcap#23803 to release-4.0
196ad55 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Apr 2, 2021
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #23819

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Apr 2, 2021
@AilinKid @ti-srebot
cherry pick pingcap#23803 to release-5.0-rc
c94790f 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Apr 2, 2021
Closed
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc in PR #23820

@ti-srebot ti-srebot mentioned this pull request Apr 2, 2021
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0 in PR #23823

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Apr 2, 2021
@AilinKid @ti-srebot
cherry pick pingcap#23803 to release-5.0
3e8d756 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
ti-chi-bot pushed a commit that referenced this pull request Apr 19, 2021
@ti-srebot
plugin: fix audit plugin will cause tidb panic (#23803) (#23823)
653b6ee
ti-chi-bot pushed a commit that referenced this pull request May 10, 2021
@ti-srebot
plugin: fix audit plugin will cause tidb panic (#23803) (#23819)
47deb7a
@ti-srebot ti-srebot mentioned this pull request Jun 9, 2021
Closed
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jun 9, 2021
@AilinKid @ti-srebot
cherry pick pingcap#23803 to release-3.0
a69dba1 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-3.0 in PR #25275

@ti-srebot
Copy link
Contributor

cherry pick to release-3.1 failed

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

@djshow832 djshow832 djshow832 left review comments

@wshwsh12 wshwsh12 Awaiting requested review from wshwsh12

@lysu lysu Awaiting requested review from lysu

Assignees
No one assigned
Labels
needs-cherry-pick-release-5.0 sig/sql-infra SIG: SQL Infra size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

plugin: audit plugin panic with nil pointer dereference
6 participants
@AilinKid @lysu @wshwsh12 @ti-chi-bot @djshow832 @ti-srebot