Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions pkg/connector/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,14 @@ func (lc *LineClient) Connect(ctx context.Context) {
}
}

lc.wg.Add(4)
go lc.syncChats(ctx)
// Create/sync group portals before message prefetching or SSE polling can
// deliver messages. Otherwise an existing LINE group whose Matrix room
// doesn't exist yet may be created by the first message, which makes the
// sender's existing membership look like a fresh join.
lc.wg.Add(1)
lc.syncChats(ctx)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Synchronous syncChats blocks startup of every other login. mautrix's bridge.StartLogins (bridge.go:373) calls login.Client.Connect(ctx) for each login in sequence; previously all four sync goroutines were launched non-blocking and Connect returned almost immediately. Now Connect waits for syncChats to finish queueing every group plus up to 30s for portal creation (waitForGroupPortalCreates). For a single-login bridge that's fine; on a multi-login install all subsequent logins are delayed by that per-login window. If you want the ordering guarantee without blocking startup, you can keep syncChats in a goroutine and have prefetchMessages + pollLoop wait on a shared exsync.Event set when waitForGroupPortalCreates returns.


lc.wg.Add(3)
Comment on lines +301 to +304

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'wg\.(Add|Done|Wait)\(|func \(lc \*LineClient\) (syncChats|syncDMChats|prefetchMessages|pollLoop|Disconnect)\b' pkg/connector

Repository: beeper/line

Length of output: 6067


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the startup / shutdown flow around Connect and Disconnect.
sed -n '250,360p' pkg/connector/client.go
printf '\n---\n'
sed -n '420,450p' pkg/connector/client.go
printf '\n---\n'
sed -n '1080,1145p' pkg/connector/sync.go
printf '\n---\n'
rg -n -C2 'Connect\(context\.Background\(\)\)|Disconnect\(\)|wg\.Wait\(|wg\.Add\(' pkg/connector

Repository: beeper/line

Length of output: 9939


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the startup / shutdown flow around Connect and Disconnect.
sed -n '250,360p' pkg/connector/client.go
printf '\n---\n'
sed -n '420,450p' pkg/connector/client.go
printf '\n---\n'
sed -n '1080,1145p' pkg/connector/sync.go
printf '\n---\n'
rg -n -C2 'Connect\(context\.Background\(\)\)|Disconnect\(\)|wg\.Wait\(|wg\.Add\(' pkg/connector

Repository: beeper/line

Length of output: 9939


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the startup / shutdown flow around Connect and Disconnect.
sed -n '250,360p' pkg/connector/client.go
printf '\n---\n'
sed -n '420,450p' pkg/connector/client.go
printf '\n---\n'
sed -n '1080,1145p' pkg/connector/sync.go
printf '\n---\n'
rg -n -C2 'Connect\(context\.Background\(\)\)|Disconnect\(\)|wg\.Wait\(|wg\.Add\(' pkg/connector

Repository: beeper/line

Length of output: 9939


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the startup / shutdown flow around Connect and Disconnect.
sed -n '250,360p' pkg/connector/client.go
printf '\n---\n'
sed -n '420,450p' pkg/connector/client.go
printf '\n---\n'
sed -n '1080,1145p' pkg/connector/sync.go
printf '\n---\n'
rg -n -C2 'Connect\(context\.Background\(\)\)|Disconnect\(\)|wg\.Wait\(|wg\.Add\(' pkg/connector

Repository: beeper/line

Length of output: 9939


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the startup / shutdown flow around Connect and Disconnect.
sed -n '250,360p' pkg/connector/client.go
printf '\n---\n'
sed -n '420,450p' pkg/connector/client.go
printf '\n---\n'
sed -n '1080,1145p' pkg/connector/sync.go
printf '\n---\n'
rg -n -C2 'Connect\(context\.Background\(\)\)|Disconnect\(\)|wg\.Wait\(|wg\.Add\(' pkg/connector

Repository: beeper/line

Length of output: 9939


Reserve all four WaitGroup slots before syncChats in pkg/connector/client.go:301-307. Disconnect() can return after syncChats drops the counter to zero, before the later Add(3), so the remaining goroutines may start after shutdown. Use Add(4) up front to cover all startup work.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/connector/client.go` around lines 301 - 304, The startup WaitGroup
accounting in connector client setup is split around syncChats, which can let
Disconnect() observe zero before the remaining goroutines are registered. Update
the initialization flow in Client startup so all four slots are reserved before
calling syncChats, using the existing lc.wg and syncChats logic, then remove the
later partial Add call to keep shutdown and startup ordering safe.

go lc.syncDMChats(ctx)
go lc.prefetchMessages(ctx)
go lc.pollLoop(ctx)
Expand Down
51 changes: 46 additions & 5 deletions pkg/connector/handle_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,31 @@ func groupDecryptLogContext(evt *zerolog.Event, msg *line.Message, chatMID strin
return evt
}

type messageWithChatInfo struct {
*simplevent.Message[line.Message]
GetChatInfoFunc func(ctx context.Context, portal *bridgev2.Portal) (*bridgev2.ChatInfo, error)
}

var _ bridgev2.RemoteChatResyncWithInfo = (*messageWithChatInfo)(nil)

func (evt *messageWithChatInfo) GetChatInfo(ctx context.Context, portal *bridgev2.Portal) (*bridgev2.ChatInfo, error) {
if evt.GetChatInfoFunc == nil {
return nil, nil
}
return evt.GetChatInfoFunc(ctx, portal)
}

func (lc *LineClient) getChatInfoForIncomingMessage(ctx context.Context, portal *bridgev2.Portal, chatMid string) (*bridgev2.ChatInfo, error) {
info, err := lc.GetChatInfo(ctx, portal)
if err != nil {
return nil, err
}
if isChatMID(chatMid) {
lc.stripRemoteMembersFromInitialChatInfo(info)
}
return info, nil
}

func (lc *LineClient) queueIncomingMessage(msg *line.Message, opType int) {
// Only process known content types; skip system messages (group created, member invited, etc.)
if !isBridgeableContentType(msg) {
Expand All @@ -87,29 +112,45 @@ func (lc *LineClient) queueIncomingMessage(msg *line.Message, opType int) {
return
}

senderID := makeUserID(msg.From)

portalIDStr := portalMIDForMessage(msg, opType)
portalKey := networkid.PortalKey{ID: makePortalID(portalIDStr), Receiver: lc.UserLogin.ID}
ts := lc.parseMessageTimestamp(msg)

lc.ensureGroupMessageSenderKnown(portalIDStr, msg.From, ts)

senderID := makeUserID(msg.From)
bodyText, unwrappedText := lc.decryptMessageBody(msg, portalIDStr, opType)
ts := lc.parseMessageTimestamp(msg)

lc.UserLogin.Bridge.QueueRemoteEvent(lc.UserLogin, &simplevent.Message[line.Message]{
messageEvent := &simplevent.Message[line.Message]{
EventMeta: simplevent.EventMeta{
Type: bridgev2.RemoteEventMessage,
LogContext: func(c zerolog.Context) zerolog.Context { return c.Str("msg_id", msg.ID) },
PortalKey: portalKey,
CreatePortal: true,
Sender: bridgev2.EventSender{Sender: senderID, IsFromMe: OperationType(opType) == OpSendMessage},
Timestamp: ts,
PreHandleFunc: func(ctx context.Context, portal *bridgev2.Portal) {
lc.hiddenJoinGroupMessageSender(ctx, portal, portalIDStr, msg.From, ts)
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Duplicate first-message join paths. On line 119 we call ensureGroupMessageSenderKnown(...) (queues a RemoteEventChatInfoChange if addGroupMembersToCache returned true), and here we also wire PreHandleFunc → hiddenJoinGroupMessageSender(...), which synchronously calls portal.ProcessChatInfoChange. For the first message from a new sender both fire and both go through syncParticipants; for every subsequent message the PreHandle one still does the full GetPowerLevels+GetMembers work (see other comment). Consider keeping only one of the two, or at minimum have hiddenJoinGroupMessageSender honour addGroupMembersToCache's return value so it short-circuits after the first message.

Comment on lines +132 to +134

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Avoid emitting a hidden join for every group message.

PreHandleFunc runs per queued message, while hiddenJoinGroupMessageSender unconditionally calls ProcessChatInfoChange for any non-own group sender. This can repeatedly send identical hidden membership joins for active group members. Please gate this on whether addGroupMembersToCache actually inserted a new member, or check the cache before calling the helper.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/connector/handle_message.go` around lines 132 - 134, The group-message
pre-handler is emitting hidden joins on every queued message because
PreHandleFunc always calls hiddenJoinGroupMessageSender, which then
unconditionally triggers ProcessChatInfoChange for non-own senders. Update the
flow in handle_message.go so hiddenJoinGroupMessageSender only runs when
addGroupMembersToCache actually adds a new member, or add a cache membership
check before calling it, using the existing hiddenJoinGroupMessageSender and
addGroupMembersToCache paths to prevent duplicate hidden join events.

},
Data: *msg,
ID: networkid.MessageID(msg.ID),
ConvertMessageFunc: func(ctx context.Context, portal *bridgev2.Portal, intent bridgev2.MatrixAPI, data line.Message) (*bridgev2.ConvertedMessage, error) {
return lc.convertLineMessage(ctx, portal, intent, data, bodyText, unwrappedText)
},
})
}

var remoteEvent bridgev2.RemoteEvent = messageEvent
if isChatMID(portalIDStr) {
remoteEvent = &messageWithChatInfo{
Message: messageEvent,
GetChatInfoFunc: func(ctx context.Context, portal *bridgev2.Portal) (*bridgev2.ChatInfo, error) {
return lc.getChatInfoForIncomingMessage(ctx, portal, portalIDStr)
},
}
}

lc.UserLogin.Bridge.QueueRemoteEvent(lc.UserLogin, remoteEvent)
}

// isBridgeableContentType reports whether an inbound LINE message should be
Expand Down
Loading
Loading