- When working with the storage arrays for CustomQuery and TableContent, use thread locking to ensure data safety while the table is loading. This should fix intermittent loading and reload issues, including part of #463 and should address Issue #482

4 files changed, 134 insertions, 48 deletions

diff --git a/Source/CustomQuery.h b/Source/CustomQuery.h
index d9963799..0cbb6c0c 100644
--- a/Source/CustomQuery.h
+++ b/Source/CustomQuery.h
@@ -122,10 +122,12 @@
 	NSString *helpHTMLTemplate;
 
 	NSMutableArray *fullResult;
+	pthread_mutex_t fullResultLock;
 	NSInteger fullResultCount;
 	NSArray *cqColumnDefinition;
 	NSString *lastExecutedQuery;
 
+	BOOL isWorking;
 	BOOL tableReloadAfterEditing;
 	BOOL queryIsTableSorter;
 	BOOL isDesc;
diff --git a/Source/CustomQuery.m b/Source/CustomQuery.m
index 826258cd..522fb5d0 100644
--- a/Source/CustomQuery.m
+++ b/Source/CustomQuery.m
@@ -807,7 +807,9 @@
 	// Remove all items from the table
 	fullResultCount = 0;
 	[customQueryView performSelectorOnMainThread:@selector(noteNumberOfRowsChanged) withObject:nil waitUntilDone:YES];
+	pthread_mutex_lock(&fullResultLock);
 	[fullResult removeAllObjects];
+	pthread_mutex_unlock(&fullResultLock);
 
 	// Set up an autorelease pool for row processing
 	dataLoadingPool = [[NSAutoreleasePool alloc] init];
@@ -815,8 +817,10 @@
 	// Loop through the result rows as they become available
 	while (tempRow = [theResult fetchNextRowAsArray]) {
 
+		pthread_mutex_lock(&fullResultLock);
 		NSMutableArrayAddObject(fullResult, [NSMutableArray arrayWithArray:tempRow]);
 		fullResultCount++;
+		pthread_mutex_unlock(&fullResultLock);
 
 		// Update the count of rows processed
 		rowsProcessed++;
@@ -1344,16 +1348,34 @@
 - (void)tableView:(CMCopyTable *)aTableView willDisplayCell:(id)cell forTableColumn:(NSTableColumn*)aTableColumn row:(NSInteger)rowIndex
 {	
 	if (aTableView == customQueryView) {
-		
-		// Perform various result set checks to prevent crashes
-		if ((fullResultCount == 0) || (rowIndex > fullResultCount)) return;
-		
+
 		// For NULL cell's display the user's NULL value placeholder in grey to easily distinguish it from other values 
 		if ([cell respondsToSelector:@selector(setTextColor:)]) {
-		
-			id value = NSArrayObjectAtIndex(NSArrayObjectAtIndex(fullResult, rowIndex), [[aTableColumn identifier] intValue]);
+			NSUInteger columnIndex = [[aTableColumn identifier] intValue];
+			id theValue = nil;
+
+			// While the table is being loaded, additional validation is required - data
+			// locks must be used to avoid crashes, and indexes higher than the available
+			// rows or columns may be requested.  Use gray to show loading in these cases.
+			if (isWorking) {
+				pthread_mutex_lock(&fullResultLock);
+				if (rowIndex < fullResultCount) {
+					NSMutableArray *rowData = NSArrayObjectAtIndex(fullResult, rowIndex);
+					if (columnIndex < [rowData count]) {
+						theValue = NSArrayObjectAtIndex(rowData, columnIndex);
+					}
+				}
+				pthread_mutex_unlock(&fullResultLock);
+
+				if (!theValue) {
+					[cell setTextColor:[NSColor lightGrayColor]];
+					return;
+				}
+			} else {
+				theValue = NSArrayObjectAtIndex(NSArrayObjectAtIndex(fullResult, rowIndex), columnIndex);
+			}
 			
-			[cell setTextColor:[value isNSNull] ? [NSColor lightGrayColor] : [NSColor blackColor]];
+			[cell setTextColor:[theValue isNSNull] ? [NSColor lightGrayColor] : [NSColor blackColor]];
 		}
 	}
 }
@@ -1364,19 +1386,35 @@
 - (id)tableView:(NSTableView *)aTableView objectValueForTableColumn:(NSTableColumn *)aTableColumn row:(NSInteger)rowIndex
 {
 	if (aTableView == customQueryView) {
+		NSUInteger columnIndex = [[aTableColumn identifier] intValue];
+		id theValue = nil;
+
+		// While the table is being loaded, additional validation is required - data
+		// locks must be used to avoid crashes, and indexes higher than the available
+		// rows or columns may be requested.  Return "..." to indicate loading in these
+		// cases.
+		if (isWorking) {
+			pthread_mutex_lock(&fullResultLock);
+			if (rowIndex < fullResultCount) {
+				NSMutableArray *rowData = NSArrayObjectAtIndex(fullResult, rowIndex);
+				if (columnIndex < [rowData count]) {
+					theValue = NSArrayObjectAtIndex(rowData, columnIndex);
+				}
+			}
+			pthread_mutex_unlock(&fullResultLock);
+
+			if (!theValue) return @"...";
+		} else {
+			theValue = NSArrayObjectAtIndex(NSArrayObjectAtIndex(fullResult, rowIndex), columnIndex);
+		}
 		
-		// Perform various result set checks to prevent crashes
-		if ((fullResultCount == 0) || (rowIndex > fullResultCount)) return nil;
-		
-		id value = NSArrayObjectAtIndex(NSArrayObjectAtIndex(fullResult, rowIndex), [[aTableColumn identifier] intValue]);
-		
-		if ([value isKindOfClass:[NSData class]])
-			return [value shortStringRepresentationUsingEncoding:[mySQLConnection encoding]];
+		if ([theValue isKindOfClass:[NSData class]])
+			return [theValue shortStringRepresentationUsingEncoding:[mySQLConnection encoding]];
 
-		if ([value isNSNull])
+		if ([theValue isNSNull])
 			return [prefs objectForKey:SPNullValue];
 
-	    return value;
+	    return theValue;
 	}
 	else {
 		return @"";
@@ -2607,6 +2645,7 @@
  */
 - (void) startDocumentTaskForTab:(NSNotification *)aNotification
 {
+	isWorking = YES;
 
 	// Only proceed if this view is selected.
 	if (![[tableDocumentInstance selectedToolbarItemIdentifier] isEqualToString:MAIN_TOOLBAR_CUSTOM_QUERY])
@@ -2623,6 +2662,7 @@
  */
 - (void) endDocumentTaskForTab:(NSNotification *)aNotification
 {
+	isWorking = NO;
 
 	// Only proceed if this view is selected.
 	if (![[tableDocumentInstance selectedToolbarItemIdentifier] isEqualToString:MAIN_TOOLBAR_CUSTOM_QUERY])
diff --git a/Source/TableContent.h b/Source/TableContent.h
index e3a9bd50..e675d528 100644
--- a/Source/TableContent.h
+++ b/Source/TableContent.h
@@ -68,6 +68,8 @@
 	MCPConnection *mySQLConnection;
 
 	BOOL _mainNibLoaded;
+	BOOL isWorking;
+	pthread_mutex_t tableValuesLock;
 
 	NSString *selectedTable, *usedQuery;
 	NSMutableArray *tableValues, *dataColumns, *keys, *oldRow;
@@ -98,6 +100,7 @@
 
 // Table loading methods and information
 - (void) loadTable:(NSString *)aTable;
+- (void) clearTableValues;
 - (void) loadTableValues;
 - (NSString *) tableFilterString;
 - (void) updateCountText;
diff --git a/Source/TableContent.m b/Source/TableContent.m
index 7a6eb9cb..06fc9dce 100644
--- a/Source/TableContent.m
+++ b/Source/TableContent.m
@@ -59,6 +59,8 @@
 {
 	if ((self == [super init])) {
 		_mainNibLoaded = NO;
+		isWorking = NO;
+		pthread_mutex_init(&tableValuesLock, NULL);
 
 		tableValues      = [[NSMutableArray alloc] init];
 		tableRowsCount = 0;
@@ -214,10 +216,11 @@
 			[tableContentView removeTableColumn:NSArrayObjectAtIndex([tableContentView tableColumns], 0)];
 		}
 
-		// Empty the stored data arrays
+		// Empty the stored data arrays, including emptying the tableValues array
+		// by ressignment for thread safety.
 		tableRowsCount = 0;
 		previousTableRowsCount = 0;
-		[tableValues removeAllObjects];
+		[self clearTableValues];
 		[tableContentView reloadData];
 		isFiltered = NO;
 		isLimited = NO;
@@ -441,10 +444,11 @@
 	[copyButton setEnabled:NO];
 	[removeButton setEnabled:NO];
 
-	// Reset the table store if required - basically if the table is being changed
+	// Reset the table store if required - basically if the table is being changed,
+	// reassigning before emptying for thread safety.
 	if (!previousTableRowsCount) {
 		tableRowsCount = 0;
-		[tableValues removeAllObjects];
+		[self clearTableValues];
 	}
 
 	// Trigger a data refresh
@@ -480,6 +484,22 @@
 }
 
 /**
+ * Remove all items from the current table value store.  Do this by
+ * reassigning the tableValues store and releasing the old location,
+ * while setting thread safety flags.
+ */
+- (void) clearTableValues
+{
+	NSMutableArray *tableValuesTransition;
+
+	tableValuesTransition = tableValues;
+	pthread_mutex_lock(&tableValuesLock);
+	tableValues = [[NSMutableArray alloc] init];
+	pthread_mutex_unlock(&tableValuesLock);
+	[tableValuesTransition release];
+}
+ 
+/**
  * Reload the table data without reconfiguring the tableView,
  * using filters and limits as appropriate.
  * Will not refresh the table view itself.
@@ -625,6 +645,7 @@
 
 	// Loop through the result rows as they become available
 	while (tempRow = [theResult fetchNextRowAsArray]) {
+		pthread_mutex_lock(&tableValuesLock);
 
 		if (rowsProcessed < previousTableRowsCount) {
 			NSMutableArrayReplaceObject(tableValues, rowsProcessed, [NSMutableArray arrayWithArray:tempRow]);
@@ -643,6 +664,8 @@
 		}
 		rowsProcessed++;
 
+		pthread_mutex_unlock(&tableValuesLock);
+
 		// Update the task interface as necessary
 		if (!isFiltered) {
 			if (rowsProcessed < targetRowCount) {
@@ -674,7 +697,9 @@
 
 	// If the reloaded table is shorter than the previous table, remove the extra values from the storage
 	if (tableRowsCount < [tableValues count]) {
+		pthread_mutex_lock(&tableValuesLock);
 		[tableValues removeObjectsInRange:NSMakeRange(tableRowsCount, [tableValues count] - tableRowsCount)];
+		pthread_mutex_unlock(&tableValuesLock);
 	}
 
 	[tableContentView performSelectorOnMainThread:@selector(reloadData) withObject:nil waitUntilDone:NO];
@@ -974,7 +999,7 @@
 	// Reset and reload data using the new filter settings
 	previousTableRowsCount = 0;
 	tableRowsCount = 0;
-	[tableValues removeAllObjects];
+	[self clearTableValues];
 	[self loadTableValues];
 	[tableContentView scrollPoint:NSMakePoint(0.0, 0.0)];
 
@@ -2530,20 +2555,27 @@
 
 - (id)tableView:(CMCopyTable *)aTableView objectValueForTableColumn:(NSTableColumn *)aTableColumn row:(int)rowIndex
 {
-
-	// In some loading situations, where the table is being redrawn while a load operation is in process on a background
-	// thread, an index higher than the available rows/columns may be requested.  Return "..." to indicate loading in these
-	// cases - when the load completes all table data will be redrawn.
 	NSUInteger columnIndex = [[aTableColumn identifier] intValue];
-	if (rowIndex >= tableRowsCount) return @"...";
-	NSMutableArray *rowData = [NSArrayObjectAtIndex(tableValues, rowIndex) retain];
-	if (!rowData || columnIndex >= [rowData count]) {
-			if (rowData) [rowData release];
-			return @"...";
-	}
+	id theValue = nil;
+
+	// While the table is being loaded, additional validation is required - data
+	// locks must be used to avoid crashes, and indexes higher than the available
+	// rows or columns may be requested.  Return "..." to indicate loading in these
+	// cases.
+	if (isWorking) {
+		pthread_mutex_lock(&tableValuesLock);
+		if (rowIndex < tableRowsCount) {
+			NSMutableArray *rowData = NSArrayObjectAtIndex(tableValues, rowIndex);
+			if (columnIndex < [rowData count]) {
+				theValue = NSArrayObjectAtIndex(rowData, columnIndex);
+			}
+		}
+		pthread_mutex_unlock(&tableValuesLock);
 
-	id theValue = NSArrayObjectAtIndex(rowData, columnIndex);
-	[rowData release];
+		if (!theValue) return @"...";
+	} else {
+		theValue = NSArrayObjectAtIndex(NSArrayObjectAtIndex(tableValues, rowIndex), columnIndex);
+	}
 
 	if ([theValue isNSNull])
 		return [prefs objectForKey:SPNullValue];
@@ -2564,23 +2596,29 @@
 {
 	if (![cell respondsToSelector:@selector(setTextColor:)]) return;
 
-	// In some loading situations, where the table is being redrawn while a load operation is in process on a background
-	// thread, an index higher than the available rows/columns may be requested.  Return gray to indicate loading in these
-	// cases - when the load completes all table data will be redrawn.
 	NSUInteger columnIndex = [[aTableColumn identifier] intValue];
-	if (rowIndex >= tableRowsCount) {
-		[cell setTextColor:[NSColor lightGrayColor]];
-		return;
-	}
-	NSMutableArray *rowData = [NSArrayObjectAtIndex(tableValues, rowIndex) retain];
-	if (!rowData || columnIndex >= [rowData count]) {
-		if (rowData) [rowData release];
-		[cell setTextColor:[NSColor lightGrayColor]];
-		return;
-	}
+	id theValue = nil;
+
+	// While the table is being loaded, additional validation is required - data
+	// locks must be used to avoid crashes, and indexes higher than the available
+	// rows or columns may be requested.  Use gray to indicate loading in these cases.
+	if (isWorking) {
+		pthread_mutex_lock(&tableValuesLock);
+		if (rowIndex < tableRowsCount) {
+			NSMutableArray *rowData = NSArrayObjectAtIndex(tableValues, rowIndex);
+			if (columnIndex < [rowData count]) {
+				theValue = NSArrayObjectAtIndex(rowData, columnIndex);
+			}
+		}
+		pthread_mutex_unlock(&tableValuesLock);
 
-	id theValue = NSArrayObjectAtIndex(rowData, columnIndex);
-	[rowData release];
+		if (!theValue) {
+			[cell setTextColor:[NSColor lightGrayColor]];
+			return;
+		}
+	} else {
+		theValue = NSArrayObjectAtIndex(NSArrayObjectAtIndex(tableValues, rowIndex), columnIndex);
+	}
 
 	// If user wants to edit 'cell' set text color to black and return to avoid
 	// writing in gray if value was NULL
@@ -2894,6 +2932,7 @@
  */
 - (void) startDocumentTaskForTab:(NSNotification *)aNotification
 {
+	isWorking = YES;
 
 	// Only proceed if this view is selected.
 	if (![[tableDocumentInstance selectedToolbarItemIdentifier] isEqualToString:MAIN_TOOLBAR_TABLE_CONTENT])
@@ -2915,6 +2954,7 @@
  */
 - (void) endDocumentTaskForTab:(NSNotification *)aNotification
 {
+	isWorking = NO;
 
 	// Only proceed if this view is selected.
 	if (![[tableDocumentInstance selectedToolbarItemIdentifier] isEqualToString:MAIN_TOOLBAR_TABLE_CONTENT])
@@ -3055,6 +3095,7 @@
 	[[NSNotificationCenter defaultCenter] removeObserver:self];
 
 	[tableValues release];
+	pthread_mutex_destroy(&tableValuesLock);
 	[dataColumns release];
 	[oldRow release];
 	if (contentFilters) [contentFilters release];