summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com>2023-11-30 11:32:45 +0000
committerLibravatar GitHub <noreply@github.com>2023-11-30 12:32:45 +0100
commit5fd2e427bb1c8fb9f7f27a60d60283f16a976875 (patch)
tree789da987c9b0f4d3fc906b9b2d6f0fc8e245d169
parent[chore] Re-add indexes, rename account actions indexes (#2401) (diff)
downloadgotosocial-5fd2e427bb1c8fb9f7f27a60d60283f16a976875.tar.xz
[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
-rw-r--r--internal/federation/dereferencing/status.go19
-rw-r--r--internal/processing/workers/fromfediapi.go20
2 files changed, 14 insertions, 25 deletions
diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go
index d05875edb..8a8ec60b1 100644
--- a/internal/federation/dereferencing/status.go
+++ b/internal/federation/dereferencing/status.go
@@ -251,7 +251,11 @@ func (d *Dereferencer) enrichStatusSafely(
) (*gtsmodel.Status, ap.Statusable, bool, error) {
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
// is required by the checks below for existing tags, media etc.
if err := d.state.DB.PopulateStatus(ctx, status); err != nil {
@@ -266,9 +270,6 @@ func (d *Dereferencer) enrichStatusSafely(
unlock = doOnce(unlock)
defer unlock()
- // This is a NEW status (to us).
- isNew := (status.ID == "")
-
// Perform status enrichment with passed vars.
latest, apubStatus, err := d.enrichStatus(ctx,
requestUser,
@@ -292,7 +293,15 @@ func (d *Dereferencer) enrichStatusSafely(
// otherwise this indicates WE
// enriched the status.
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
// in a call to db.Put(Status). Look again in DB by URI.
diff --git a/internal/processing/workers/fromfediapi.go b/internal/processing/workers/fromfediapi.go
index 9558bf8b7..a8da50819 100644
--- a/internal/processing/workers/fromfediapi.go
+++ b/internal/processing/workers/fromfediapi.go
@@ -19,7 +19,6 @@ package workers
import (
"context"
- "net/url"
"codeberg.org/gruf/go-kv"
"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)
}
- 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 != "" {
// Interaction counts changed on the replied status; uncache the
// prepared version from all timelines. The status dereferencer