Session not completely cleaned #1185

New Issue

zeripath · 2019-01-06T15:50:53Z

zeripath commented

2019-01-06 15:50:53 +00:00

Hi!

In Gitea at https://github.com/go-gitea/gitea/blob/master/models/issue.go#L1349

func Issues(opts *IssuesOptions) ([]*Issue, error) {
	sess := x.NewSession()
	defer sess.Close()

	if err := opts.setupSession(sess); err != nil {
		return nil, err
	}
	sortIssuesSession(sess, opts.SortType)

	issues := make([]*Issue, 0, setting.UI.IssuePagingNum)
	if err := sess.Find(&issues); err != nil {
		return nil, fmt.Errorf("Find: %v", err)
	}

	if err := IssueList(issues).LoadAttributes(); err != nil {
		return nil, fmt.Errorf("LoadAttributes: %v", err)
	}

	return issues, nil
}

If we change the LoadAttributes function to perform its lookups within the same session the results are not the same as those without it, because it appears that at least some of the setup performed in opts.setupSession is not reset following the Find above.

Although this doesn't present a problem at the moment because there is no transaction created and no changes are made to the database, the lack of a complete reset could cause unpredictable heisenbugs elsewhere.

Hi! In Gitea at https://github.com/go-gitea/gitea/blob/master/models/issue.go#L1349 ```go func Issues(opts *IssuesOptions) ([]*Issue, error) { sess := x.NewSession() defer sess.Close() if err := opts.setupSession(sess); err != nil { return nil, err } sortIssuesSession(sess, opts.SortType) issues := make([]*Issue, 0, setting.UI.IssuePagingNum) if err := sess.Find(&issues); err != nil { return nil, fmt.Errorf("Find: %v", err) } if err := IssueList(issues).LoadAttributes(); err != nil { return nil, fmt.Errorf("LoadAttributes: %v", err) } return issues, nil } ``` If we change the `LoadAttributes `function to perform its lookups within the same session the results are not the same as those without it, because it appears that at least some of the setup performed in `opts.setupSession` is not reset following the `Find` above. Although this doesn't present a problem at the moment because there is no transaction created and no changes are made to the database, the lack of a complete reset could cause unpredictable heisenbugs elsewhere.

zeripath commented

2019-01-07 19:36:27 +00:00

OK, so the reason this appears to fail in Gitea is that the failure happens because issues.loadAttachments(sess) calls issues.loadLabels(sess):

func (issues IssueList) loadLabels(e Engine) error {
	if len(issues) == 0 {
		return nil
	}

	type LabelIssue struct {
		Label      *Label      `xorm:"extends"`
		IssueLabel *IssueLabel `xorm:"extends"`
	}

	var issueLabels = make(map[int64][]*Label, len(issues)*3)
	var issueIDs = issues.getIssueIDs()
	var left = len(issueIDs)
	for left > 0 {
		var limit = defaultMaxInSize
		if left < limit {
			limit = left
		}
		rows, err := e.Table("label").
			Join("LEFT", "issue_label", "issue_label.label_id = label.id").
			In("issue_label.issue_id", issueIDs[:limit]).
			Asc("label.name").
			Rows(new(LabelIssue))
		if err != nil {
			return err
		}

		for rows.Next() {
			var labelIssue LabelIssue
			err = rows.Scan(&labelIssue)
			if err != nil {
				rows.Close()
				return err
			}
			issueLabels[labelIssue.IssueLabel.IssueID] = append(issueLabels[labelIssue.IssueLabel.IssueID], labelIssue.Label)
		}
		rows.Close()
		left = left - limit
		issueIDs = issueIDs[limit:]
	}

	for _, issue := range issues {
		issue.Labels = issueLabels[issue.ID]
	}
	return nil
}

Now it appears that the line:

err = rows.Scan(&labelIssue)

Causes xorm to issue:

SELECT `id`, `repo_id`, `name`, `description`, `color`, `num_issues`, `num_closed_issues`, `id`, `issue_id`, `label_id` FROM `label_issue` WHERE `id` IN (?) []interface {}{0}

The table label_issue doesn't exist.

Looking at rows.go:

// Scan row record to bean properties
func (rows *Rows) Scan(bean interface{}) error {
	if rows.lastError != nil {
		return rows.lastError
	}

	if !rows.NoTypeCheck && reflect.Indirect(reflect.ValueOf(bean)).Type() != rows.beanType {
		return fmt.Errorf("scan arg is incompatible type to [%v]", rows.beanType)
	}

	if err := rows.session.statement.setRefBean(bean); err != nil {
		return err
	}

	scanResults, err := rows.session.row2Slice(rows.rows, rows.fields, bean)
	if err != nil {
		return err
	}

	dataStruct := rValue(bean)
	_, err = rows.session.slice2Bean(scanResults, rows.fields, bean, &dataStruct, rows.session.statement.RefTable)
	if err != nil {
		return err
	}

	return rows.session.executeProcessors()
}

I suspect the problem lies in the last line: rows.session.executeProcessors()

OK, so the reason this appears to fail in Gitea is that the failure happens because `issues.loadAttachments(sess)` calls `issues.loadLabels(sess)`: ```go func (issues IssueList) loadLabels(e Engine) error { if len(issues) == 0 { return nil } type LabelIssue struct { Label *Label `xorm:"extends"` IssueLabel *IssueLabel `xorm:"extends"` } var issueLabels = make(map[int64][]*Label, len(issues)*3) var issueIDs = issues.getIssueIDs() var left = len(issueIDs) for left > 0 { var limit = defaultMaxInSize if left < limit { limit = left } rows, err := e.Table("label"). Join("LEFT", "issue_label", "issue_label.label_id = label.id"). In("issue_label.issue_id", issueIDs[:limit]). Asc("label.name"). Rows(new(LabelIssue)) if err != nil { return err } for rows.Next() { var labelIssue LabelIssue err = rows.Scan(&labelIssue) if err != nil { rows.Close() return err } issueLabels[labelIssue.IssueLabel.IssueID] = append(issueLabels[labelIssue.IssueLabel.IssueID], labelIssue.Label) } rows.Close() left = left - limit issueIDs = issueIDs[limit:] } for _, issue := range issues { issue.Labels = issueLabels[issue.ID] } return nil } ``` Now it appears that the line: ```go err = rows.Scan(&labelIssue) ``` Causes xorm to issue: ```sql SELECT `id`, `repo_id`, `name`, `description`, `color`, `num_issues`, `num_closed_issues`, `id`, `issue_id`, `label_id` FROM `label_issue` WHERE `id` IN (?) []interface {}{0} ``` The table label_issue doesn't exist. Looking at `rows.go`: ```go // Scan row record to bean properties func (rows *Rows) Scan(bean interface{}) error { if rows.lastError != nil { return rows.lastError } if !rows.NoTypeCheck && reflect.Indirect(reflect.ValueOf(bean)).Type() != rows.beanType { return fmt.Errorf("scan arg is incompatible type to [%v]", rows.beanType) } if err := rows.session.statement.setRefBean(bean); err != nil { return err } scanResults, err := rows.session.row2Slice(rows.rows, rows.fields, bean) if err != nil { return err } dataStruct := rValue(bean) _, err = rows.session.slice2Bean(scanResults, rows.fields, bean, &dataStruct, rows.session.statement.RefTable) if err != nil { return err } return rows.session.executeProcessors() } ``` I suspect the problem lies in the last line: `rows.session.executeProcessors()`

lunny commented

2019-01-13 15:04:31 +00:00

This should be a bug of xorm and I think it's on Table("table_name").Rows(). The table name haven't been used.

This should be a bug of xorm and I think it's on `Table("table_name").Rows()`. The table name haven't been used.

lunny added this to the 1.0.0 milestone 2020-03-04 02:21:03 +00:00

lunny referenced a pull request that will close this issue

2020-03-06 08:51:11 +00:00

Fix rows bug #1576

lunny closed this issue

2020-03-06 12:08:37 +00:00

Sign in to join this conversation.