HTTP 500 Error and site unresponsive when using assignment notifications via slack webhook

[jimpson]

• Gitea version (or commit ref): 1.7.1
• Git version: 2.14.1
• Operating system: Windows Server 2012 R2
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant
• Log gist:

2019/02/15 11:12:52 [...s/issue_assignees.go:141 ChangeAssignee()] [E] PrepareWebhooks [is_pull: true, remove_assignee: false]: GetSlackPayload: read tcp SERVER_IP_ADDRESS_REDACTED->SQL_IP_ADDRESS_REDACTED:1433: i/o timeout
2019/02/15 11:13:05 [...user/notification.go:33 GetNotificationCount()] [E] GetNotificationCount: sql: expected 5 destination arguments in Scan, not 1

Possibly Related To

#5639 Assigning a PR causes the server to hang

Description

When a slack webhook is defined to send notifications related to pull requests, specifically assignment events, the Gitea server takes a long time (sql timeout length) to respond and once it does, there appears to no longer be any data being pulled into the web view. I have tested this with all other events and there does not seem to be any issues with the webhook and PRs other than the PR assignment event. It should be noted that after restarting the server the assignee on the PR is sometimes populated and sometimes missing. I'm guessing there is a deadlock on the SQL side of things, but that goes beyond my expertise.
...

Steps To Reproduce

• Setup a slack webhook and select the "Pull Request" custom event
• Open a PR and assign it to a user
• Note the unresponsiveness
• After ~30 seconds refresh the page
• Note the appearance of missing data
• Restart the server
• Go back to the PR that was opened previously
• Note if the assignment was successful or failed

Screenshots

[zeripath]

This is suspicious for a sqlite deadlock bug.

I think a few log lines above the error might be helpful too. I suspect that the log lines you've reported might already be deadlocked but I'll look into them.

[lunny]

@zeripath but his database is mssql.

[zeripath]

Oh yeah.

Still suspicious for an SQL deadlock bug though...

[zeripath]

Yup there's a deadlock:

https://github.com/go-gitea/gitea/blob/master/models/webhook_slack.go#L304

Calls list, err := MakeAssigneeList(&Issue{ID: p.PullRequest.ID}) which is a database function, whilst still within the transaction opened in:

https://github.com/go-gitea/gitea/blob/master/models/issue_assignees.go#L133

[zeripath]

Ceterum autem censeo x exempla monstrabit esse delendam.

[zeripath]

OK, it's not just Slack, Discord and Dingtalk have the problem too.

Now strangely there is an Assignees member in api.PullRequest which has been there for approximately as long as these lines - so I'm not sure why this wasn't used - perhaps it wasn't there...

Now, there's a valid question about whether webhook event generation should occur within the same transaction and the same request as the actual assignment but I'm going to ignore that for the moment and fix the issue at hand.