Skip to content

Commit d4b9703

Browse files
committed
fix incoming messages being added twice to the virtualizingcollection's cache lines
Off-by-one errors are fun! Since incoming messages are being saved before adding, we were fetching them from the database when creating the cache line AND appending it.
1 parent de8e15a commit d4b9703

File tree

2 files changed

+15
-3
lines changed

2 files changed

+15
-3
lines changed

Signal-Windows/Controls/VirtualizedMessagesCollection.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,28 @@ public object this[int index]
6767

6868
public object SyncRoot => this;
6969

70+
/// <summary>
71+
/// "Adds" a SignalMessageContainer to this virtualized collection.</summary>
72+
/// <remarks>
73+
/// The method may (if incoming) or may not (if outgoing) already be present in the database, so we explicitly insert at the correct position in the cache line.
74+
/// Count is mapped to the SignalConversation's MessagesCount, so callers must update appropriately before calling this method, and no async method must be called in between.</remarks>
75+
/// <param name="value">The object to add to the VirtualizedMessagesCollection.</param>
76+
/// <returns>The position into which the new element was inserted, or -1 to indicate that the item was not inserted into the collection.</returns>
7077
public int Add(object value)
7178
{
7279
var message = value as SignalMessageContainer;
7380
int inpageIndex = message.Index % PAGE_SIZE;
7481
int pageIndex = GetPageIndex(message.Index);
82+
Debug.WriteLine($"VirtualizedCollection.Add Id={message.Message.Id} Index={message.Index} PageIndex={pageIndex} InpageIndex={inpageIndex} ");
7583
if (!Cache.ContainsKey(pageIndex))
7684
{
7785
Cache[pageIndex] = SignalDBContext.GetMessagesLocked(Conversation, pageIndex * PAGE_SIZE, PAGE_SIZE);
7886
}
79-
Cache[pageIndex].Add(message);
87+
Cache[pageIndex].Insert(inpageIndex, message);
88+
if (Cache[pageIndex].IndexOf(message) != inpageIndex || message.Index != Count-1)
89+
{
90+
throw new InvalidOperationException("VirtualizedCollection is in an inconsistent state!");
91+
}
8092
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, message, message.Index));
8193
return message.Index;
8294
}

Signal-Windows/Storage/DB.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -780,7 +780,7 @@ public static void SaveMessageLocked(SignalMessage message)
780780

781781
public static List<SignalMessageContainer> GetMessagesLocked(SignalConversation thread, int startIndex, int count)
782782
{
783-
Debug.WriteLine($"GetMessagesLocked {thread.ThreadId} {startIndex} {count}");
783+
Debug.WriteLine($"GetMessagesLocked {thread.ThreadId} Skip({startIndex}) Take({count})");
784784
lock (DBLock)
785785
{
786786
using (var ctx = new SignalDBContext())
@@ -794,7 +794,7 @@ public static List<SignalMessageContainer> GetMessagesLocked(SignalConversation
794794
.Skip(startIndex)
795795
.Take(count);
796796

797-
var containers = new List<SignalMessageContainer>();
797+
var containers = new List<SignalMessageContainer>(count);
798798
foreach (var message in messages)
799799
{
800800
containers.Add(new SignalMessageContainer(message, startIndex));

0 commit comments

Comments
 (0)