mirrors
/
gotosocial
mirror of
1
Fork 0
Code Issues Releases Activity

[bugfix] always go through status parent dereferencing on isNew, even on data-race (#2402) 

* no need to deref status author account, will already be deref'd during previous getStatusByAP{IRI,Model}()

* don't unset the isNew flag on dereference data race

* improved code comment
This commit is contained in:
kim 2023-11-30 11:32:45 +00:00 committed by GitHub
parent f9ba0df726
commit 5fd2e427bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
internal/federation/dereferencing/status.go
View File

@ -251,7 +251,11 @@ func (d *Dereferencer) enrichStatusSafely(
) (*gtsmodel.Status, ap.Statusable, bool, error) { ) (*gtsmodel.Status, ap.Statusable, bool, error) {
uriStr := status.URI uriStr := status.URI
if status.ID != "" { var isNew bool
// Check if this is a new status (to us).
if isNew = (status.ID == ""); !isNew {
// This is an existing status, first try to populate it. This // This is an existing status, first try to populate it. This
// is required by the checks below for existing tags, media etc. // is required by the checks below for existing tags, media etc.
if err := d.state.DB.PopulateStatus(ctx, status); err != nil { if err := d.state.DB.PopulateStatus(ctx, status); err != nil {
@ -266,9 +270,6 @@ func (d *Dereferencer) enrichStatusSafely(
unlock = doOnce(unlock) unlock = doOnce(unlock)
defer unlock() defer unlock()
// This is a NEW status (to us).
isNew := (status.ID == "")
// Perform status enrichment with passed vars. // Perform status enrichment with passed vars.
latest, apubStatus, err := d.enrichStatus(ctx, latest, apubStatus, err := d.enrichStatus(ctx,
requestUser, requestUser,
@ -292,7 +293,15 @@ func (d *Dereferencer) enrichStatusSafely(
// otherwise this indicates WE // otherwise this indicates WE
// enriched the status. // enriched the status.
apubStatus = nil apubStatus = nil
isNew = false
// We leave 'isNew' set so that caller
// still dereferences parents, otherwise
// the version we pass back may not have
// these attached as inReplyTos yet (since
// those happen OUTSIDE federator lock).
//
// TODO: performance-wise, this won't be
// great. should improve this if we can!
// DATA RACE! We likely lost out to another goroutine // DATA RACE! We likely lost out to another goroutine
// in a call to db.Put(Status). Look again in DB by URI. // in a call to db.Put(Status). Look again in DB by URI.

20
internal/processing/workers/fromfediapi.go
View File

@ -19,7 +19,6 @@ package workers
import ( import (
"context" "context"
"net/url"
"codeberg.org/gruf/go-kv" "codeberg.org/gruf/go-kv"
"codeberg.org/gruf/go-logger/v2/level" "codeberg.org/gruf/go-logger/v2/level"
@ -169,25 +168,6 @@ func (p *fediAPI) CreateStatus(ctx context.Context, fMsg messages.FromFediAPI) e
return gtserror.Newf("error extracting status from federatorMsg: %w", err) return gtserror.Newf("error extracting status from federatorMsg: %w", err)
} }
if status.Account == nil || status.Account.IsRemote() {
// Either no account attached yet, or a remote account.
// Both situations we need to parse account URI to fetch it.
accountURI, err := url.Parse(status.AccountURI)
if err != nil {
return gtserror.Newf("error parsing account uri: %w", err)
}
// Ensure that account for this status has been deref'd.
status.Account, _, err = p.federate.GetAccountByURI(
ctx,
fMsg.ReceivingAccount.Username,
accountURI,
)
if err != nil {
return gtserror.Newf("error getting account by uri: %w", err)
}
}
if status.InReplyToID != "" { if status.InReplyToID != "" {
// Interaction counts changed on the replied status; uncache the // Interaction counts changed on the replied status; uncache the
// prepared version from all timelines. The status dereferencer // prepared version from all timelines. The status dereferencer