Skip to content

Commit 3312e33

Browse files
6.4.19-b3 (#1590)
- Don't try to retrieve messages older than our autodelete timeout, if active - Added more debug logging to catch deadlock
2 parents b960b87 + 071f867 commit 3312e33

File tree

5 files changed

+78
-9
lines changed

5 files changed

+78
-9
lines changed

Monal/Classes/DataLayer.m

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,11 +357,19 @@ -(NSMutableDictionary*) readStateForAccount:(NSNumber*) accountNo
357357
return nil;
358358
NSString* query = @"SELECT state from account where account_id=?";
359359
NSArray* params = @[accountNo];
360+
DDLogVerbose(@"Outside of transaction: Reading account state: %@", accountNo);
361+
[MLSQLite debugTransactions];
360362
NSData* data = (NSData*)[self.db idReadTransaction:^{
363+
DDLogVerbose(@"Inside of transaction: Reading account state: %@", accountNo);
361364
return [self.db executeScalar:query andArguments:params];
362365
}];
366+
DDLogVerbose(@"After transaction: Reading account state: %@ --> %@", accountNo, data);
363367
if(data)
364-
return [HelperTools unserializeData:data];
368+
{
369+
NSMutableDictionary* retval = [HelperTools unserializeData:data];
370+
DDLogVerbose(@"After unserializeData: Reading account state: %@", accountNo);
371+
return retval;
372+
}
365373
return nil;
366374
}
367375

Monal/Classes/MLMessageProcessor.m

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,28 @@ +(MLMessage* _Nullable) processMessage:(XMPPMessage*) messageNode andOuterMessag
117117
return nil;
118118
}
119119

120-
if([messageNode check:@"/<type=headline>/{http://jabber.org/protocol/pubsub#event}event"])
120+
if([messageNode check:@"/<type=headline>/{http://jabber.org/protocol/pubsub#event}event"] && !isMLhistory)
121121
{
122122
[account.pubsub handleHeadlineMessage:messageNode];
123123
return nil;
124124
}
125125

126+
//ignore messages older than our autodelete time
127+
//(history backscrolling ones AND normal ones, both would otherwise disappear again immediately afterwards)
128+
//do this before handling the message stanza: only non-MLhistory pubsub messages and error-type stanzas should be processed
129+
//regardless of their age (but pubsub messages only if they aren't MLhistory ones)
130+
NSInteger autodeleteInterval = [[HelperTools defaultsDB] integerForKey:@"AutodeleteInterval"];
131+
if(autodeleteInterval > 0)
132+
{
133+
NSDate* pastDate = [NSDate dateWithTimeIntervalSinceNow:-autodeleteInterval];
134+
NSDate* messageTimestamp = [messageNode findFirst:@"{urn:xmpp:delay}delay@stamp|datetime"];
135+
if(messageTimestamp != nil && [messageTimestamp compare:pastDate] == NSOrderedAscending)
136+
{
137+
DDLogInfo(@"Ignoring incoming message being too old (probably an MLhistory one)...");
138+
return nil;
139+
}
140+
}
141+
126142
//ignore messages from our own device, see this github issue: https://github.com/monal-im/Monal/issues/941
127143
if(![messageNode check:@"/<type=groupchat>"] && !isMLhistory && [messageNode.from isEqualToString:account.connectionProperties.identity.fullJid] && [messageNode.toUser isEqualToString:account.connectionProperties.identity.jid])
128144
return nil;

Monal/Classes/MLSQLite.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ typedef BOOL (^monal_sqlite_bool_operations_t)(void);
1616

1717
@interface MLSQLite : NSObject
1818

19+
+(void) debugTransactions;
20+
1921
+(id) sharedInstanceForFile:(NSString*) dbFile;
2022

2123
-(void) voidReadTransaction:(monal_void_block_t) operations;

Monal/Classes/MLSQLite.m

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ +(void) initialize
3838
DDLogInfo(@"sqlite initialize: using mysql lib version: %s", sqlite3_libversion());
3939
}
4040

41+
+(void) debugTransactions
42+
{
43+
NSMutableDictionary* threadData = [[NSThread currentThread] threadDictionary];
44+
DDLogVerbose(@"threadData: %@", threadData);
45+
DDLogVerbose(@"currentTransactions: %@", currentTransactions);
46+
}
47+
4148
//every thread gets its own instance having its own db connection
4249
//this allows for concurrent reads/writes
4350
+(id) sharedInstanceForFile:(NSString*) dbFile

Monal/Classes/xmpp.m

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ -(BOOL) idle
500500
//only check unacked count if needed (this makes sure we don't hold the state lock unnecessarily and block the main thread)
501501
if(retval)
502502
{
503+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
503504
@synchronized(_stateLockObject) {
504505
NSUInteger unacked = [self.unAckedStanzas count];
505506
if(unacked)
@@ -508,6 +509,7 @@ -(BOOL) idle
508509
unackedCount = @(unacked);
509510
}
510511
}
512+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
511513
}
512514
_lastIdleState = retval;
513515
DDLogVerbose(@("%@ --> Idle check:\n"
@@ -1634,13 +1636,16 @@ -(void) sendPing:(double) timeout
16341636

16351637
-(void) addSmacksHandler:(monal_void_block_t) handler
16361638
{
1639+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
16371640
@synchronized(_stateLockObject) {
16381641
[self addSmacksHandler:handler forValue:self.lastOutboundStanza];
16391642
}
1643+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
16401644
}
16411645

16421646
-(void) addSmacksHandler:(monal_void_block_t) handler forValue:(NSNumber*) value
16431647
{
1648+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
16441649
@synchronized(_stateLockObject) {
16451650
if([value integerValue] < [self.lastOutboundStanza integerValue])
16461651
{
@@ -1652,10 +1657,12 @@ -(void) addSmacksHandler:(monal_void_block_t) handler forValue:(NSNumber*) value
16521657
NSDictionary* dic = @{@"value":value, @"handler":handler};
16531658
[_smacksAckHandler addObject:dic];
16541659
}
1660+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
16551661
}
16561662

16571663
-(void) resendUnackedStanzas
16581664
{
1665+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
16591666
@synchronized(_stateLockObject) {
16601667
DDLogInfo(@"Resending unacked stanzas...");
16611668
NSMutableArray* sendCopy = [[NSMutableArray alloc] initWithArray:self.unAckedStanzas];
@@ -1669,12 +1676,14 @@ -(void) resendUnackedStanzas
16691676
DDLogInfo(@"Done resending unacked stanzas...");
16701677
[self persistState];
16711678
}
1679+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
16721680
}
16731681

16741682
-(void) resendUnackedMessageStanzasOnly:(NSMutableArray*) stanzas
16751683
{
16761684
if(stanzas)
16771685
{
1686+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
16781687
@synchronized(_stateLockObject) {
16791688
DDLogWarn(@"Resending unacked message stanzas only...");
16801689
NSMutableArray* sendCopy = [[NSMutableArray alloc] initWithArray:stanzas];
@@ -1692,11 +1701,13 @@ -(void) resendUnackedMessageStanzasOnly:(NSMutableArray*) stanzas
16921701
//or contain all the resent stanzas (e.g. only resume failed)
16931702
[self persistState];
16941703
}
1704+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
16951705
}
16961706
}
16971707

16981708
-(BOOL) shouldTriggerSyncErrorForImportantUnackedOutgoingStanzas
16991709
{
1710+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
17001711
@synchronized(_stateLockObject) {
17011712
DDLogInfo(@"Checking for important unacked stanzas...");
17021713
for(NSDictionary* dic in self.unAckedStanzas)
@@ -1711,13 +1722,15 @@ -(BOOL) shouldTriggerSyncErrorForImportantUnackedOutgoingStanzas
17111722
return YES;
17121723
}
17131724
}
1725+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
17141726
//no important stanzas found
17151727
return NO;
17161728
}
17171729

17181730
-(BOOL) removeAckedStanzasFromQueue:(NSNumber*) hvalue
17191731
{
17201732
NSMutableArray* ackHandlerToCall = [[NSMutableArray alloc] initWithCapacity:[_smacksAckHandler count]];
1733+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
17211734
@synchronized(_stateLockObject) {
17221735
//sanity check
17231736
MLAssert(([self.unAckedStanzas count]+[self.lastHandledOutboundStanza unsignedIntValue])==[self.lastOutboundStanza unsignedIntValue], @"Calculated outgoing stanza count and counted one differ!", (@{
@@ -1808,6 +1821,7 @@ -(BOOL) removeAckedStanzasFromQueue:(NSNumber*) hvalue
18081821
}
18091822
[_smacksAckHandler removeObjectsInArray:ackHandlerToCall];
18101823
}
1824+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
18111825

18121826
//call registered smacksAckHandler that got sorted out
18131827
for(NSDictionary* dic in ackHandlerToCall)
@@ -1823,6 +1837,7 @@ -(void) requestSMAck:(BOOL) force
18231837
{
18241838
//caution: this could be called from sendQueue, too!
18251839
MLXMLNode* rNode;
1840+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
18261841
@synchronized(_stateLockObject) {
18271842
unsigned long unackedCount = (unsigned long)[self.unAckedStanzas count];
18281843
if(self.accountState >= kStateInitStarted && self.connectionProperties.supportsSM3 &&
@@ -1835,6 +1850,7 @@ -(void) requestSMAck:(BOOL) force
18351850
else
18361851
DDLogDebug(@"no smacks request, there is nothing pending or a request already in flight...");
18371852
}
1853+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
18381854
if(rNode)
18391855
[self send:rNode];
18401856
}
@@ -1856,11 +1872,13 @@ -(void) sendSMAck:(BOOL) queuedSend
18561872
return;
18571873

18581874
NSDictionary* dic;
1875+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
18591876
@synchronized(_stateLockObject) {
18601877
dic = @{
18611878
@"h":[NSString stringWithFormat:@"%@",self.lastHandledInboundStanza],
18621879
};
18631880
}
1881+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
18641882
MLXMLNode* aNode = [[MLXMLNode alloc] initWithElement:@"a" andNamespace:@"urn:xmpp:sm:3" withAttributes:dic andChildren:@[] andData:nil];
18651883
if(queuedSend)
18661884
[self send:aNode];
@@ -1897,13 +1915,15 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
18971915
if(h==nil)
18981916
return [self invalidXMLError];
18991917

1918+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
19001919
@synchronized(_stateLockObject) {
19011920
//remove acked messages
19021921
[self removeAckedStanzasFromQueue:h];
19031922

19041923
self.smacksRequestInFlight = NO; //ack returned
19051924
[self requestSMAck:NO]; //request ack again (will only happen if queue is not empty)
19061925
}
1926+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
19071927
}
19081928
else if([parsedStanza check:@"/{jabber:client}presence"] && self.accountState >= kStateInitStarted)
19091929
{
@@ -2155,6 +2175,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
21552175
{
21562176
//wrap everything in lock instead of writing the boolean result into a temp var because incrementLastHandledStanza
21572177
//is wrapped in this lock, too (and we don't call anything else here)
2178+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
21582179
@synchronized(_stateLockObject) {
21592180
if(_runningMamQueries[[outerMessageNode findFirst:@"{urn:xmpp:mam:2}result@queryid"]] == nil)
21602181
{
@@ -2165,6 +2186,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
21652186
return;
21662187
}
21672188
}
2189+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
21682190

21692191
//create a new XMPPMessage node instead of only a MLXMLNode because messages have some convenience properties and methods
21702192
messageNode = [[XMPPMessage alloc] initWithXMPPMessage:[outerMessageNode findFirst:@"{urn:xmpp:mam:2}result/{urn:xmpp:forward:0}forwarded/{jabber:client}message"]];
@@ -2380,6 +2402,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
23802402
else if([parsedStanza check:@"/{urn:xmpp:sm:3}enabled"] && self.accountState == kStateBound)
23812403
{
23822404
NSMutableArray* stanzas;
2405+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
23832406
@synchronized(_stateLockObject) {
23842407
//save old unAckedStanzas queue before it is cleared
23852408
stanzas = self.unAckedStanzas;
@@ -2396,6 +2419,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
23962419
//persist these changes (streamID and initSM3)
23972420
[self persistState];
23982421
}
2422+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
23992423

24002424
//init session and query disco, roster etc.
24012425
[self initSession];
@@ -2423,11 +2447,13 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
24232447
[[MLNotificationQueue currentQueue] postNotificationName:kMLSessionInitNotice object:self];
24242448
[self accountStatusChanged];
24252449

2450+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
24262451
@synchronized(_stateLockObject) {
24272452
//remove already delivered stanzas and resend the (still) unacked ones
24282453
[self removeAckedStanzasFromQueue:h];
24292454
[self resendUnackedStanzas];
24302455
}
2456+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
24312457

24322458
//publish new csi and last active state (but only do so when not in an extension
24332459
//because the last active state does not change when inside an extension)
@@ -2444,6 +2470,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
24442470
//ping all mucs to check if we are still connected (XEP-0410)
24452471
[self.mucProcessor pingAllMucs];
24462472

2473+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
24472474
@synchronized(_stateLockObject) {
24482475
//signal finished catchup if our current outgoing stanza counter is acked, this introduces an additional roundtrip to make sure
24492476
//all stanzas the *server* wanted to replay have been received, too
@@ -2479,6 +2506,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
24792506
}
24802507
}];
24812508
}
2509+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
24822510

24832511
//initialize stanza counter for statistics
24842512
[self initCatchupStats];
@@ -2489,6 +2517,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
24892517

24902518
__block BOOL error = NO;
24912519
self.resuming = NO;
2520+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
24922521
@synchronized(_stateLockObject) {
24932522
//invalidate stream id
24942523
self.streamID = nil;
@@ -2500,6 +2529,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
25002529
//persist these changes
25012530
[self persistState];
25022531
}
2532+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
25032533

25042534
//don't try to bind, if removeAckedStanzasFromQueue returned an error (it will trigger a reconnect in these cases)
25052535
if(!error)
@@ -3311,6 +3341,7 @@ -(void) handleFeaturesAfterAuth:(MLXMLNode*) parsedStanza
33113341
}
33123342

33133343
MLXMLNode* resumeNode = nil;
3344+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
33143345
@synchronized(_stateLockObject) {
33153346
//test if smacks is supported and allows resume
33163347
if(self.connectionProperties.supportsSM3 && self.streamID)
@@ -3323,6 +3354,7 @@ -(void) handleFeaturesAfterAuth:(MLXMLNode*) parsedStanza
33233354
self.resuming = YES; //this is needed to distinguish a failed smacks resume and a failed smacks enable later on
33243355
}
33253356
}
3357+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
33263358
if(resumeNode)
33273359
[self send:resumeNode];
33283360
else
@@ -3785,6 +3817,7 @@ -(void) readState
37853817

37863818
-(void) realReadState
37873819
{
3820+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
37883821
@synchronized(_stateLockObject) {
37893822
DDLogVerbose(@"%@ --> realReadState before: used/available memory: %.3fMiB / %.3fMiB)...", self.accountNo, [HelperTools report_memory], (CGFloat)os_proc_available_memory() / 1048576);
37903823
NSMutableDictionary* dic = [[DataLayer sharedInstance] readStateForAccount:self.accountNo];
@@ -3815,16 +3848,14 @@ -(void) realReadState
38153848
self.isDoingFullReconnect = isDoingFullReconnect.boolValue;
38163849
}
38173850

3818-
@synchronized(_stateLockObject) {
3819-
//invalidate corrupt smacks states (this could potentially loose messages, but hey, the state is corrupt anyways)
3820-
if(self.lastHandledInboundStanza == nil || self.lastHandledOutboundStanza == nil || self.lastOutboundStanza == nil || !self.unAckedStanzas)
3821-
{
3851+
//invalidate corrupt smacks states (this could potentially loose messages, but hey, the state is corrupt anyways)
3852+
if(self.lastHandledInboundStanza == nil || self.lastHandledOutboundStanza == nil || self.lastOutboundStanza == nil || !self.unAckedStanzas)
3853+
{
38223854
#ifndef IS_ALPHA
3823-
[self initSM3];
3855+
[self initSM3];
38243856
#else
3825-
@throw [NSException exceptionWithName:@"RuntimeError" reason:@"corrupt smacks state" userInfo:dic];
3857+
@throw [NSException exceptionWithName:@"RuntimeError" reason:@"corrupt smacks state" userInfo:dic];
38263858
#endif
3827-
}
38283859
}
38293860

38303861
NSDictionary* persistentIqHandlers = [dic objectForKey:@"iqHandlers"];
@@ -3985,6 +4016,7 @@ -(void) realReadState
39854016

39864017
DDLogVerbose(@"%@ --> realReadState after: used/available memory: %.3fMiB / %.3fMiB)...", self.accountNo, [HelperTools report_memory], (CGFloat)os_proc_available_memory() / 1048576);
39874018
}
4019+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
39884020
}
39894021

39904022
+(NSMutableDictionary*) invalidateState:(NSDictionary*) dic
@@ -4020,6 +4052,7 @@ -(void) incrementLastHandledStanzaWithDelayedReplay:(BOOL) delayedReplay
40204052
//don't ack messages twice
40214053
if(delayedReplay)
40224054
return;
4055+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
40234056
@synchronized(_stateLockObject) {
40244057
if(self.connectionProperties.supportsSM3)
40254058
{
@@ -4030,11 +4063,13 @@ -(void) incrementLastHandledStanzaWithDelayedReplay:(BOOL) delayedReplay
40304063
}
40314064
[self persistState]; //make sure we persist our state, even if smacks is not supported
40324065
}
4066+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
40334067
}
40344068

40354069
-(void) initSM3
40364070
{
40374071
//initialize smacks state
4072+
DDLogVerbose(@"Before @synchronized of _stateLockObject...");
40384073
@synchronized(_stateLockObject) {
40394074
self.lastHandledInboundStanza = [NSNumber numberWithInteger:0];
40404075
self.lastHandledOutboundStanza = [NSNumber numberWithInteger:0];
@@ -4044,6 +4079,7 @@ -(void) initSM3
40444079
_smacksAckHandler = [NSMutableArray new];
40454080
DDLogDebug(@"initSM3 done");
40464081
}
4082+
DDLogVerbose(@"After @synchronized of _stateLockObject...");
40474083
}
40484084

40494085
-(void) bindResource:(NSString*) resource

0 commit comments

Comments
 (0)