Add a safety check within [SPCopyTable shouldUseFieldEditorForRow:col…

…umn:] for use when loading tables, which may improve Issue #1925 and Issue #1902.
sequelpro · Jun 16, 2014 · 690c2fa83a5a3138bf4aa0fa862565c833b72d2a · 690c2fa
1 parent ae23907
commit 690c2fa83a5a3138bf4aa0fa862565c833b72d2a
Unified Split

Showing with 35 additions and 11 deletions.

+3 −2 Source/SPCopyTable.h

+20 −2 Source/SPCopyTable.m

+3 −3 Source/SPCustomQuery.m

+7 −2 Source/SPTableContentDataSource.m

+2 −2 Source/SPTableContentDelegate.m
diff --git a/Source/SPCopyTable.h b/Source/SPCopyTable.h
@@ -183,15 +183,16 @@ extern NSInteger SPEditCopyAsSQL;
 - (BOOL)isCellComplex;

 /*!
-	@method	 shouldUseFieldEditorForRow:column:
+	@method	 shouldUseFieldEditorForRow:column:useLock:
 	@abstract   Determine whether to trigger sheet editing or in-cell editing for a cell
 	@discussion Checks the column data type, and the cell contents if necessary, to check
 		the most appropriate editing type.
 	@param	 rowIndex The row in the table the cell is present in
 	@param	 colIndex The *original* column in the table the cell is present in (ie pre-reordering)
+	@param	 dataLock An optional pthread_mutex_t lock to use when checking the data
 	@result	 YES if sheet editing should be used, NO otherwise.
 */
- (BOOL)shouldUseFieldEditorForRow:(NSUInteger)rowIndex column:(NSUInteger)colIndex;
+- (BOOL)shouldUseFieldEditorForRow:(NSUInteger)rowIndex column:(NSUInteger)colIndex checkWithLock:(pthread_mutex_t *)dataLock;

 - (IBAction)executeBundleItemForDataTable:(id)sender;

diff --git a/Source/SPCopyTable.m b/Source/SPCopyTable.m
@@ -52,6 +52,7 @@
 #import "SPDatabaseContentViewDelegate.h"

 #import <SPMySQL/SPMySQL.h>
+#import "pthread.h"

 NSInteger SPEditMenuCopy            = 2001;
 NSInteger SPEditMenuCopyWithColumns = 2002;
@@ -1186,7 +1187,7 @@ - (void)keyDown:(NSEvent *)theEvent
 * Determine whether to use the sheet for editing; do so if the multipleLineEditingButton is enabled,
 * or if the column was a blob or a text, or if it contains linebreaks.
 */
- (BOOL)shouldUseFieldEditorForRow:(NSUInteger)rowIndex column:(NSUInteger)colIndex
+- (BOOL)shouldUseFieldEditorForRow:(NSUInteger)rowIndex column:(NSUInteger)colIndex checkWithLock:(pthread_mutex_t *)dataLock
 {
 	// Retrieve the column definition
 	NSDictionary *columnDefinition = [[(id <SPDatabaseContentViewDelegate>)[self delegate] dataColumnDefinitions] objectAtIndex:colIndex];
@@ -1203,7 +1204,24 @@ - (BOOL)shouldUseFieldEditorForRow:(NSUInteger)rowIndex column:(NSUInteger)colIn
 	if (isBlob && ![columnType isEqualToString:@"enum"]) return YES;

 	// Otherwise, check the cell value for newlines.
-	id cellValue = [tableStorage cellDataAtRow:rowIndex column:colIndex];
+	id cellValue = nil;
+
+	// If a data lock was supplied, use it and perform additional checks for safety
+	if (dataLock) {
+		pthread_mutex_lock(dataLock);
+
+		if (rowIndex < [tableStorage count] && colIndex < [tableStorage columnCount]) {
+			cellValue = [tableStorage cellDataAtRow:rowIndex column:colIndex];
+		}
+
+		pthread_mutex_unlock(dataLock);
+
+		if (!cellValue) return YES;
+
+	// Otherwise grab the value directly
+	} else {
+		cellValue = [tableStorage cellDataAtRow:rowIndex column:colIndex];
+	}

 	if ([cellValue isKindOfClass:[NSData class]]) {
 		cellValue = [[[NSString alloc] initWithData:cellValue encoding:[mySQLConnection stringEncoding]] autorelease];
diff --git a/Source/SPCustomQuery.m b/Source/SPCustomQuery.m
@@ -2121,7 +2121,7 @@ - (void)tableView:(NSTableView *)aTableView setObjectValue:(id)anObject forTable

 		// If the current cell should have been edited in a sheet, do nothing - field closing will have already
 		// updated the field.
-		if ([customQueryView shouldUseFieldEditorForRow:rowIndex column:[[aTableColumn identifier] integerValue]]) {
+		if ([customQueryView shouldUseFieldEditorForRow:rowIndex column:[[aTableColumn identifier] integerValue] checkWithLock:NULL]) {
 			return;
 		}

@@ -2449,7 +2449,7 @@ - (BOOL)tableView:(NSTableView *)aTableView shouldEditTableColumn:(NSTableColumn
 						|| [[columnDefinition objectForKey:@"typegrouping"] isEqualToString:@"blobdata"]);

 		// Open the editing sheet if required
-		if ([customQueryView shouldUseFieldEditorForRow:rowIndex column:[[aTableColumn identifier] integerValue]])
+		if ([customQueryView shouldUseFieldEditorForRow:rowIndex column:[[aTableColumn identifier] integerValue] checkWithLock:NULL])
 		{
 			if (fieldEditor) [fieldEditor release], fieldEditor = nil;
 			fieldEditor = [[SPFieldEditorController alloc] init];
@@ -3851,7 +3851,7 @@ - (BOOL)control:(NSControl *)control textShouldBeginEditing:(NSText *)aFieldEdit
 	isFieldEditable = shouldBeginEditing;

 	// Open the field editor sheet if required
-	if ([customQueryView shouldUseFieldEditorForRow:row column:column])
+	if ([customQueryView shouldUseFieldEditorForRow:row column:column checkWithLock:NULL])
 	{

 		[customQueryView setFieldEditorSelectedRange:[aFieldEditor selectedRange]];
diff --git a/Source/SPTableContentDataSource.m b/Source/SPTableContentDataSource.m
@@ -129,7 +129,12 @@ - (id)tableView:(SPCopyTable *)tableView objectValueForTableColumn:(NSTableColum
 					return [NSString stringWithFormat:@"0x%@", [value dataToHexString]];
 				}

-				if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:columnIndex]) {
+				pthread_mutex_t *fieldEditorCheckLock = NULL;
+				if (isWorking) {
+					fieldEditorCheckLock = &tableValuesLock;
+				}
+
+				if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:columnIndex checkWithLock:fieldEditorCheckLock]) {
 					return [value shortStringRepresentationUsingEncoding:[mySQLConnection stringEncoding]];
 				}
 				
@@ -167,7 +172,7 @@ - (void)tableView:(NSTableView *)tableView setObjectValue:(id)object forTableCol
 			
 			// If the current cell should have been edited in a sheet, do nothing - field closing will have already
 			// updated the field.
-			if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:[[tableColumn identifier] integerValue]]) {
+			if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:[[tableColumn identifier] integerValue] checkWithLock:NULL]) {
 				return;
 			}
 			
diff --git a/Source/SPTableContentDelegate.m b/Source/SPTableContentDelegate.m
@@ -262,7 +262,7 @@ - (BOOL)tableView:(NSTableView *)tableView shouldEditTableColumn:(NSTableColumn
 			}
 			
 			// Open the editing sheet if required
-			if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:[[tableColumn identifier] integerValue]]) {
+			if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:[[tableColumn identifier] integerValue] checkWithLock:NULL]) {
 				
 				// Retrieve the column definition
 				NSDictionary *columnDefinition = [cqColumnDefinition objectAtIndex:[[tableColumn identifier] integerValue]];
@@ -750,7 +750,7 @@ - (BOOL)control:(NSControl *)control textShouldBeginEditing:(NSText *)aFieldEdit
 	}
 	
 	// Open the field editor sheet if required
-	if ([tableContentView shouldUseFieldEditorForRow:row column:column])
+	if ([tableContentView shouldUseFieldEditorForRow:row column:column checkWithLock:NULL])
 	{
 		[tableContentView setFieldEditorSelectedRange:[aFieldEditor selectedRange]];