diff --git a/dat1.h b/dat1.h index f14ceed..0c67a33 100644 --- a/dat1.h +++ b/dat1.h @@ -72,6 +72,7 @@ *- */ +#include #include "hdf5.h" #include "hdf5_hl.h" #include "hds1.h" @@ -141,24 +142,20 @@ typedef enum { /* This structure contains information about an HDF5 object (group or dataset) that is common to all the locators that refer to the object. */ typedef struct Handle { - char locked; /* Non-zero if the HDF object is currently locked for - access by a single thread. Zero if the object is - unlocked. */ - pthread_t locker; /* Id for the thread that has locked the object (if - any) */ - struct Handle *parent; /* Pointer to Handle describing the parent object */ - struct Handle **children; /* Pointer to array holding pointers to Handles - for any known child objects */ - int nchild; /* The length of the "children" array */ - char *name; /* Name (cleaned) of the HDF object within its parent */ - pthread_mutex_t mutex2; /* Guards access to the values in the handle */ - -} Handle; + pthread_mutex_t mutex; /* Guards access to the values in the handle */ -/* Define functions to lock and unlock a Handle's mutex. */ -#define DAT_LOCK_MUTEX2(han) pthread_mutex_lock( &((han)->mutex2) ) -#define DAT_UNLOCK_MUTEX2(han) pthread_mutex_unlock( &((han)->mutex2) ) + int nwrite_lock; /* Number of current write locks (0 or 1) */ + pthread_t write_locker; /* ID for thread holding write lock */ + int nread_lock; /* Number of current read locks (0 or more) */ + pthread_t *read_lockers; /* Array of IDs for thread holding read locks */ + int maxreaders; /* Current size of "read_lockers" array */ + struct Handle *parent; /* Pointer to Handle describing the parent object */ + struct Handle **children;/* Pointer to array holding pointers to Handles + for any known child objects */ + int nchild; /* The length of the "children" array */ + char *name; /* Name (cleaned) of the HDF object within its parent */ +} Handle; /* Private definition of the HDS locator struct */ typedef struct LOC { @@ -429,11 +426,12 @@ dat1GetStructureDims( const HDSLoc * locator, int maxdims, hdsdim dims[], int *s hdsbool_t dat1NeedsRootName( hid_t objid, hdsbool_t wantprim, char * rootname, size_t rootnamelen, int * status ); -Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int * status ); +Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int +rdonly, int * status ); Handle *dat1EraseHandle( Handle *parent, const char *name, int * status ); Handle *dat1FreeHandle( Handle *handle ); -int dat1ValidateLocator( int checklock, const HDSLoc *loc, int *status ); -int dat1HandleLock( Handle *handle, int oper, int recurs, int *status ); +int dat1ValidateLocator( const char *func, int checklock, const HDSLoc *loc, int rdonly, int *status ); +int dat1HandleLock( Handle *handle, int oper, int recurs, int rdonly, int *status ); void dat1HandleMsg( const char *token, const Handle *handle ); /* DAT1_H_INCLUDED */ diff --git a/dat1FreeHandle.c b/dat1FreeHandle.c index ee397eb..46f7ffd 100644 --- a/dat1FreeHandle.c +++ b/dat1FreeHandle.c @@ -94,9 +94,10 @@ Handle *dat1FreeHandle( Handle *handle ) { /* Free the memory used by components of the Handle structure. */ if( handle->name ) MEM_FREE( handle->name ); if( handle->children ) MEM_FREE( handle->children ); + if( handle->read_lockers ) MEM_FREE( handle->read_lockers ); /* Destroy the mutex */ - pthread_mutex_destroy( &(handle->mutex2) ); + pthread_mutex_destroy( &(handle->mutex) ); /* Fill the handles with zeros in case any other points to the same handle exist. */ diff --git a/dat1Handle.c b/dat1Handle.c index 507be84..1df1074 100644 --- a/dat1Handle.c +++ b/dat1Handle.c @@ -13,7 +13,8 @@ * Library routine * Invocation: -* Handle *dat1Handle( const HDSLoc *parent, const char *name, int *status ); +* Handle *dat1Handle( const HDSLoc *parent, const char *name, +* int rdonly, int *status ); * Arguments: * parent = const HDSLoc * (Given) @@ -26,6 +27,14 @@ * component does in fact exist within the parent object, although * this is not checked. If "parent" is NULL, the path to the * contained file should be supplied. +* rdonly = int (Given) +* If a new Handle is created as a result of calling this function, +* it is locked for use by the current thread. If "parent" is +* non-NULL, the type of lock (read-only or read-write) is copied +* from the parent. If "parent" is NULL, the type of lock is +* specified by the "rdonly" argument. The supplied "rdonly" value +* is ignored if "parent" is non-NULL or if a pointer to an existing +* handle is returned. * status = int* (Given and Returned) * Pointer to global status. @@ -103,7 +112,8 @@ #include "dat1.h" #include "dat_err.h" -Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int * status ){ +Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int rdonly, + int * status ){ /* Local Variables; */ @@ -112,6 +122,7 @@ Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int * status ){ Handle *parent; Handle *result = NULL; int ichild; + int lock_status; /* Return immediately if an error has already occurred. */ if( *status != SAI__OK ) return result; @@ -121,7 +132,7 @@ Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int * status ){ lname = MEM_CALLOC( strlen( name ) + 1, sizeof(char) ); if( !lname ) { *status = DAT__NOMEM; - emsRep("dat1Handle", "Could not reallocate memory for the " + emsRep("dat1Handle", "Could not allocate memory for the " "component name in an HDS Handle", status ); } else { strcpy( lname, name ); @@ -175,9 +186,6 @@ Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int * status ){ parent->children[ parent->nchild-1 ] = result; } -/* Copy other information from the parent Handle. */ - result->locked = parent->locked; - result->locker = parent->locker; } /* Store the component name. Nullify "lname" to indicate the memory is @@ -188,14 +196,40 @@ Handle *dat1Handle( const HDSLoc *parent_loc, const char *name, int * status ){ /* Initialise a mutex that is used to serialise access to the values stored in the handle. */ if( *status == SAI__OK && - pthread_mutex_init( &(result->mutex2), NULL ) != 0 ) { - emsRep( " ", "Failed to initialise POSIX mutex2 for a new Handle.", + pthread_mutex_init( &(result->mutex), NULL ) != 0 ) { + *status = DAT__MUTEX; + emsRep( " ", "Failed to initialise POSIX mutex for a new Handle.", status ); } -/* Indicate the Handle is locked for use by the current thread. */ - result->locked = 1; - result->locker = pthread_self(); +/* Initialise the Handle to indicate it is currently unlocked. */ + result->nwrite_lock = 0; + result->nread_lock = 0; + result->read_lockers = NULL; + result->maxreaders = 0; + +/* If a parent was supplied, see if the current thread has a read or + write lock on the parent object. We give the same sort of lock to the + new Handle below (ignoring the supplied value for "rdonly"). */ + if( parent ) { + lock_status = dat1HandleLock( parent, 1, 0, 0, status ); + if( lock_status == 1 ) { + rdonly = 0; + } else if( lock_status == 3 ) { + rdonly = 1; + } else if( *status == SAI__OK ) { + *status = DAT__FATAL; + emsRepf( " ", "dat1Handle: Unexpected lock value (%d) for " + "object '%s' - parent of '%s' (internal HDS " + "programming error).", status, lock_status, + parent->name, name ); + } + } + +/* Lock the new Handle for use by the current thread. The type of lock + (read-only or read-write) is inherited from the parent (if there is a + parent) or supplied by the caller. */ + dat1HandleLock( result, 2, 0, rdonly, status ); } } diff --git a/dat1HandleLock.c b/dat1HandleLock.c index d577eff..9a53c4d 100644 --- a/dat1HandleLock.c +++ b/dat1HandleLock.c @@ -13,7 +13,8 @@ * Library routine * Invocation: -* int dat1HandleLock( Handle *handle, int oper, int recurs, int *status ); +* int dat1HandleLock( Handle *handle, int oper, int recurs, +* int rdonly, int *status ); * Arguments: * handle = Handle * (Given) @@ -21,21 +22,28 @@ * oper = int (Given) * Operation to be performed: * -* 1 - Return information about the current lock on the supplied Handle. -* Returns 0 if the supplied Handle is unlocked; 1 if the supplied -* Handle is locked by the current thread; 2 if the supplied Handle -* is locked by a different thread. Any child Handles are always -* ignored. -* 2 - Lock the handle for use by the current thread. Returns 0 if the -* handle is already locked by a different thread (in which case -* the request to lock the supplied Handle is ignored) and +1 -* otherwise. Any child Handles within the supplied Handle are -* left unchanged unless "recurs" is non-zero. -* 3 - Unlock the handle. Returns 0 if the handle is already locked -* by a different thread (in which case the request to unlock the -* supplied Handle is ignored) and +1 otherwise. Any child Handles +* 1 - Return information about the current locks on the supplied Handle. +* +* 0: unlocked; +* 1: locked for writing by the current thread; +* 2: locked for writing by another thread; +* 3: locked for reading by the current thread (other threads +* may also have a read lock on the Handle); +* 4: locked for reading by one or more other threads (the +* current thread does not have a read lock on the Handle); +* +* Any child Handles are always ignored. +* 2 - Lock the handle for read-write or read-only use by the current +* thread. Returns 0 if the requested lock conflicts with an +* existing lock (in which case the request to lock the supplied +* Handle is ignored) and +1 otherwise. Any child Handles within +* the supplied Handle are left unchanged unless "recurs" is +* non-zero. +* 3 - Unlock the handle. If the current thread has a lock - either +* read-write or read-only - on the Handle, it is removed. +* Otherwise the Handle is left unchanged. Any child Handles * within the supplied Handle are left unchanged unless "recurs" -* is non-zero. +* is non-zero. A value of +1 is always returned. * * recurs = int (Given) * Only used if "oper" is 2 or 3, and the returned value is non-zero @@ -43,10 +51,13 @@ * is zero, the supplied Handle is the only Handle to be modified - * any child Handles are left unchanged. If "recurs" is non-zero, any * child Handles contained within the supplied Handle are locked or -* unlocked recursively, in addition to locking or unclocking the -* supplied Handle. Any child Handles that are already locked by -* another thread are ignored and left unchanged. The returned value -* is not affected by the value of "recurs". +* unlocked recursively, in addition to locking or unlocking the +* supplied Handle. Any child Handles that cannot be locked or +* unlocked and left unchanged. The returned value is not affected +* by the value of "recurs". +* rdonly = int (Given) +* Only used if "oper" is 2. It indicates if the new lock is for +* read-only or read-write access. * status = int* (Given and Returned) * Pointer to global status. @@ -55,11 +66,32 @@ * description of the "oper" argument above. * Description: -* This function locks or unlocks the supplied Handle. When locked, -* the locking thread has exclusive access to the object (but not -* necessarily any component objects) attached to the supplied handle. +* This function locks or unlocks the supplied Handle for read-only or +* read-write access. The Handle is always in one of the following +* three mutually exclusive states: +* +* - Unlocked +* - Locked for read-write access by one and only one thread. +* - Locked for read-only access by one or more threads. +* +* When locked for read-write access, the locking thread has exclusive +* read-write access to the object attached to the supplied handle. When +* locked for read-only access, the locking thread shares read-only +* access with zero or more other threads. +* +* A request to lock or unlock an object can be propagated recursively +* to all child objects by setting "recurs" non-zero. However, if the +* attempt to lock or unlock a child fails, no error will be reported +* and the child will be left unchanged. * Notes: +* - If a thread gets a read-write lock on the handle, and +* subsequently attempts to get a read-only lock, the existing +* read-write lock will be demoted to a read-only lock. +* - If a thread gets a read-only lock on the handle, and +* subsequently attempts to get a read-write lock, the existing +* read-only lock will be promoted to a read-write lock only if +* there are no other locks on the Handle. * - "oper" values of -2 and -3 are used in place of 2 and 3 when * calling this function recursively from within itself. * - A value of zero is returned if an error has already ocurred, or @@ -72,6 +104,8 @@ * History: * 10-JUL-2017 (DSB): * Initial version +* 19-JUL-2017 (DSB): +* Added argument rdonly. * {enter_further_changes_here} * Copyright: @@ -122,12 +156,22 @@ #include "ems.h" #include "dat_err.h" -int dat1HandleLock( Handle *handle, int oper, int recurs, int *status ){ +/* The initial size for the array holding the identifiers for the threads + that have a read lock on a handle. It is also the incremement in size + when the array needs to be extended. */ +#define NTHREAD 10 + +int dat1HandleLock( Handle *handle, int oper, int recurs, int rdonly, + int *status ){ /* Local Variables; */ int ichild; int result = 0; int top_level; + pthread_t *locker; + pthread_t *rlocker; + int i; + int j; /* Check inherited status. */ if( *status != SAI__OK ) return result; @@ -145,79 +189,201 @@ int dat1HandleLock( Handle *handle, int oper, int recurs, int *status ){ /* For top-level entries to this function, we need to ensure no other thread is modifying the details in the handle, so attempt to lock the handle's mutex. */ - if( top_level ) DAT_LOCK_MUTEX2( handle ); - + if( top_level ) pthread_mutex_lock( &(handle->mutex) ); /* Return information about the current lock on the supplied Handle. - Returns 0 if the Handle is unlocked; 1 if the Handle is locked by - the current thread; 2 if the Handle is locked by a different thread. ------------------------------------------------------------------ */ if( oper == 1 ) { -/* Check the lock on the supplied handle, and set "result" accordingly. */ - if( !handle->locked ) { - result = 0; /* Supplied Handle unlocked */ +/* Default: unlocked */ - } else if( pthread_equal( handle->locker, pthread_self() )) { - result = 1; /* Supplied Handle locked by current thread */ + if( handle->nwrite_lock ) { + if( pthread_equal( handle->write_locker, pthread_self() )) { - } else { - result = 2; /* Supplied Handle locked by other thread */ +/* Locked for writing by the current thread. */ + result = 1; + } else { + +/* Locked for writing by another thread. */ + result = 2; + } + + } else if( handle->nread_lock ){ + +/* Locked for reading by one or more other threads (the current thread does + not have a read lock on the Handle). */ + result = 4; + +/* Now check to see if the current thread has a read lock, changing the + above result value if it does. */ + locker = handle->read_lockers; + for( i = 0; i < handle->nread_lock;i++,locker++ ) { + if( pthread_equal( *locker, pthread_self() )) { + +/* Locked for reading by the current thread (other threads may also have + a read lock on the Handle). */ + result = 3; + break; + } + } } -/* Lock the handle for use by the current thread. Returns 0 if the handle - is already locked by a different thread (in which case the request to lock - the supplied Handle is ignored) and 1 otherwise. +/* Lock the handle for use by the current thread. ------------------------------------------------------------------ */ } else if( oper == 2 ) { -/* If the supplied Handle is unlocked, lock it for use by the current - thread, and return 1. */ - if( !handle->locked ){ - handle->locked = 1; - handle->locker = pthread_self(); - result = 1; - -/* If the supplied Handle is already locked for use by the current - thread, just return 1. */ - } else if( pthread_equal( handle->locker, pthread_self() )) { - result = 1; +/* A read-only lock requested.... */ + if( rdonly ) { + +/* If the current thread has a read-write lock on the Handle, demote it + to a read-only lock and return 1 (success). In this case, we know + there will be no other read-locks. Otherwise if any other thread has + read-write lock, return zero (failure). */ + if( handle->nwrite_lock ) { + if( pthread_equal( handle->write_locker, pthread_self() )) { + +/* If we do not have an array in which to store read lock thread IDs, + allocate one now with room for NTHREAD locks. It will be extended as + needed. */ + if( !handle->read_lockers ) { + handle->read_lockers = MEM_CALLOC(NTHREAD,sizeof(pthread_t)); + if( !handle->read_lockers ) { + *status = DAT__NOMEM; + emsRep( "", "Could not allocate memory for HDS " + "Handle read locks list.", status ); + } + } + +/* If we now have an array, store the current thread in the first element. */ + if( handle->read_lockers ) { + handle->read_lockers[ 0 ] = pthread_self(); + handle->nread_lock = 1; + handle->nwrite_lock = 0; + result = 1; + } + } + +/* If there is no read-write lock on the Handle, add the current thread + to the list of threads that currently have a read-only lock, but only + if it is not already there. */ + } else { + +/* Set "result" to 1 if the current thread already has a read-only lock. */ + locker = handle->read_lockers; + for( i = 0; i < handle->nread_lock;i++ ) { + if( pthread_equal( *locker, pthread_self() )) { + result = 1; + break; + } + } + +/* If not, extend the read lock thread ID array if necessary, and append + the current thread ID to the end. */ + if( result == 0 ) { + handle->nread_lock++; + if( handle->maxreaders < handle->nread_lock ) { + handle->maxreaders += NTHREAD; + handle->read_lockers = MEM_REALLOC( handle->read_lockers, + handle->maxreaders*sizeof(pthread_t)); + if( !handle->read_lockers ) { + *status = DAT__NOMEM; + emsRep( "", "Could not reallocate memory for HDS " + "Handle read locks list.", status ); + } + } + + if( handle->read_lockers ) { + handle->read_lockers[ handle->nread_lock - 1 ] = pthread_self(); + +/* Indicate the read-only lock was applied successfully. */ + result = 1; + } + } + } + +/* A read-write lock requested. */ + } else { + +/* If there are currently no locks of any kind, apply the lock. */ + if( handle->nread_lock == 0 ) { + if( handle->nwrite_lock == 0 ) { + handle->write_locker = pthread_self(); + handle->nwrite_lock = 1; + result = 1; + +/* If the current thread already has a read-write lock, indicate success. */ + } else if( pthread_equal( handle->write_locker, pthread_self() )) { + result = 1; + } + +/* If there is currently only one read-only lock, and it is owned by the + current thread, then promote it to a read-write lock. */ + } else if( handle->nread_lock == 1 && + pthread_equal( handle->read_lockers[0], pthread_self() )) { + handle->nread_lock = 0; + handle->write_locker = pthread_self(); + handle->nwrite_lock = 1; + result = 1; + } } /* If required, and if the above lock operation was successful, lock any - child handles that are not currently locked by another thread. */ + child handles that can be locked. */ if( result && recurs ){ for( ichild = 0; ichild < handle->nchild; ichild++ ) { (void) dat1HandleLock( handle->children[ichild], -2, 1, - status ); + rdonly, status ); } } -/* Unlock the handle. Returns 0 if the handle is already locked by a - different thread - in which case the request to unlock the Handle - is ignored, and 1 otherwise. - ------------------------------------------------------------------ */ + + + + +/* Unlock the handle. + ----------------- */ } else if( oper == 3 ) { -/* If the supplied Handle is already unlocked, just return 1. */ - if( !handle->locked ){ - result = 1; +/* Always indicate success because we should always be able to guarantee + that the current thread does not have a lock on exit. */ + result = 1; + +/* If the current thread has a read-write lock, remove it. */ + if( handle->nwrite_lock ) { + if( pthread_equal( handle->write_locker, pthread_self() )) { + handle->nwrite_lock = 0; + } + +/* Otherwise, if the current thread has a read-only lock, remove it. */ + } else { -/* If the supplied Handle is locked for use by the current thread, - unlock it and return 1. */ - } else if( pthread_equal( handle->locker, pthread_self() )) { - handle->locked = 0; - result = 1; +/* Loop through all the threads that have read-only locks. */ + locker = handle->read_lockers; + for( i = 0; i < handle->nread_lock; i++,locker++ ) { + +/* If the current thread is found, shuffle any remaining threads down one + slot to fill the gap left by removing the current thread from the list. */ + if( pthread_equal( *locker, pthread_self() )) { + rlocker = locker + 1; + for( j = i + 1; j < handle->nread_lock; j++,locker++ ) { + *(locker++) = *(rlocker++); + } + +/* Reduce the number of read-only locks. */ + handle->nread_lock--; + break; + } + } } /* If required, and if the above unlock operation was successful, unlock any child handles that are not currently locked by another thread. */ if( result && recurs ){ for( ichild = 0; ichild < handle->nchild; ichild++ ) { - (void) dat1HandleLock( handle->children[ichild], -3, 1, + (void) dat1HandleLock( handle->children[ichild], -3, 1, 0, status ); } } @@ -233,7 +399,7 @@ int dat1HandleLock( Handle *handle, int oper, int recurs, int *status ){ /* If this is a top-level entry, unlock the Handle's mutex so that other threads can access the values in the Handle. */ - if( top_level ) DAT_UNLOCK_MUTEX2( handle ); + if( top_level ) pthread_mutex_unlock( &(handle->mutex) ); /* Return the result. */ return result; diff --git a/dat1New.c b/dat1New.c index 66e8de2..ce99425 100644 --- a/dat1New.c +++ b/dat1New.c @@ -239,7 +239,7 @@ dat1New( const HDSLoc *locator, if (*status == SAI__OK) { HDSLoc *thisloc = dat1AllocLoc( status ); if (*status == SAI__OK) { - thisloc->handle = dat1Handle( locator, cleanname, status ); + thisloc->handle = dat1Handle( locator, cleanname, 0, status ); thisloc->dataset_id = dataset_id; thisloc->group_id = group_id; thisloc->dataspace_id = dataspace_id; diff --git a/dat1ValidateLocator.c b/dat1ValidateLocator.c index b3b8f9c..3b08139 100644 --- a/dat1ValidateLocator.c +++ b/dat1ValidateLocator.c @@ -13,15 +13,26 @@ * Library routine * Invocation: -* dat1ValidateLocator( int checklock, const HDSLoc *loc, int * status ); +* dat1ValidateLocator( const char *func, int checklock, const HDSLoc *loc, +* int rdonly, int * status ); * Arguments: +* func = const char * (Given) +* Name of calling function. Used in error messages. * checklock = int (Given) * If non-zero, an error is reported if the supplied locator is not * locked by the current thread (see datLock). This check is not * performed if "checklock" is zero. * loc = HDSLoc * (Given) * Locator to validate. +* rdonly = int (Given) +* Indicates if the calling function may or may not make any +* changes to the HDF object or locator structure. If a non-zero +* value is supplied, it is assumed that the calling function will +* never make any changes to either the HDF object on disk or the +* locator structure. This determines the type of lock that the +* calling thread must have on the object (read-only or read-write) +* to avoid an error being reported by this function. * status = int* (Given and Returned) * Pointer to global status. @@ -29,7 +40,8 @@ * An error is reported if the supplied locator is not valid. This can * occur for instance if the supplied locator is a secondary locator * that has been annulled automatically as a result of the file being -* closed. +* closed. An error is also reported if the current thread does no +* have an appropriate lock on the supplied object. * Authors: * DSB: David Berry (EAO) @@ -72,41 +84,66 @@ #include "dat1.h" #include "dat_err.h" -int dat1ValidateLocator( int checklock, const HDSLoc *loc, int * status ) { +int dat1ValidateLocator( const char *func, int checklock, const HDSLoc *loc, + int rdonly, int * status ) { /* Local Variables; */ int valid; + int lock_status; /* If the locator has been annulled (e.g. due to the container file being closed when the last primary locator was annulled), report an error. */ datValid( loc, &valid, status ); if( !valid && *status == SAI__OK ) { *status = DAT__LOCIN; - emsRep(" ", "The supplied HDS locator is invalid - it may have been " + emsRepf(" ", "%s: The supplied HDS locator is invalid - it may have been " "annulled as a result of the associated file being closed.", - status ); + status, func ); } /* Report an error if there is no handle in the locator. */ if( loc && !loc->handle && *status == SAI__OK ) { *status = DAT__FATAL; datMsg( "O", loc ); - emsRep( " ", "The supplied HDS locator for '^O' has no handle (programming error).", - status ); + emsRepf( " ", "%s: The supplied HDS locator for '^O' has no handle (programming error).", + status, func ); } -/* If required, check that the object is locked by the current thread. - Do not check any child objects as these wil be checked if and when - accessed. */ +/* If required, check that the object is locked by the current thread for + the appropriate type of access. Do not check any child objects as these + will be checked if and when accessed. */ if( checklock && *status == SAI__OK ) { - if( dat1HandleLock( loc->handle, 1, 0, status ) != 1 && - *status == SAI__OK ) { + lock_status = dat1HandleLock( loc->handle, 1, 0, 0, status ); + +/* Calling function will not make any change to the object. In this case + the current thread must have a lock but the type (read-only or + read-write) does not matter. */ + if( rdonly ) { + if( lock_status != 1 && lock_status != 3 && *status == SAI__OK ) { + *status = DAT__THREAD; + datMsg( "O", loc ); + emsRepf( " ", "%s: The supplied HDS locator for '^O' cannot be used.", + status, func ); + emsRep( " ", "It has not been locked for read-only or read-write " + "access by the current thread (programming error).", + status ); + } + +/* Calling function may make changes to the object. In this case the + current thread must have a read-write lock. */ + } else if( lock_status != 1 && *status == SAI__OK ) { *status = DAT__THREAD; datMsg( "O", loc ); - emsRep( " ", "The supplied HDS locator for '^O' cannot be used.", - status ); - emsRep( " ", "It has not been locked for use by the current thread " - "(programming error).", status ); + emsRepf( " ", "%s: The supplied HDS locator for '^O' cannot be used.", + status, func ); + if( lock_status == 3 ) { + emsRep( " ", "Write-access is not allowed (the current thread " + "has locked it for read-only access - programming error).", + status ); + } else { + emsRep( " ", "It has not been locked for read-write access by the " + "current thread (programming error).", status ); + } } } diff --git a/datAlter.c b/datAlter.c index df01cf6..b016263 100644 --- a/datAlter.c +++ b/datAlter.c @@ -126,7 +126,7 @@ datAlter( HDSLoc *locator, int ndim, const hdsdim dims[], int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datAlter", 1, locator, 0, status ); if (locator->vectorized) { *status = DAT__OBJIN; diff --git a/datBasic.c b/datBasic.c index 54da48a..8ca12e6 100644 --- a/datBasic.c +++ b/datBasic.c @@ -104,9 +104,6 @@ datBasic(const HDSLoc *locator, const char *mode_c, unsigned char **pntr, if (*status == SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - *status = SAI__ERROR; emsRep("datBasic", "datBasic is not available in HDF5 interface", status); diff --git a/datCcopy.c b/datCcopy.c index bfd9b43..968d7b3 100644 --- a/datCcopy.c +++ b/datCcopy.c @@ -105,8 +105,8 @@ datCcopy(const HDSLoc *locator1, const HDSLoc *locator2, const char *name, if (*status != SAI__OK) return *status; /* Validate input locators. */ - dat1ValidateLocator( 1, locator1, status ); - dat1ValidateLocator( 1, locator2, status ); + dat1ValidateLocator( "datCcopy", 1, locator1, 1, status ); + dat1ValidateLocator( "datCcopy", 1, locator2, 0, status ); if (dat1IsStructure(locator1, status)) { diff --git a/datCell.c b/datCell.c index 7619238..da7dfc6 100644 --- a/datCell.c +++ b/datCell.c @@ -107,12 +107,13 @@ datCell(const HDSLoc *locator1, int ndim, const hdsdim subs[], hsize_t h5subs[DAT__MXDIM]; HDSLoc * thisloc = NULL; int isstruct = 0; + int rdonly; char namestr[DAT__SZNAM+1]; if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datCell", 1, locator1, 1, status ); datName(locator1, namestr, status ); @@ -204,7 +205,25 @@ datCell(const HDSLoc *locator1, int ndim, const hdsdim subs[], if (*status == SAI__OK) { /* Handle for cell's data object */ - thisloc->handle = dat1Handle( locator1, cellname, status ); + thisloc->handle = dat1Handle( locator1, cellname, 0, status ); + + /* Determine if the current thread has a read-only or read-write lock + on the parent array, referenced by the supplied locator. */ + rdonly = ( dat1HandleLock( locator1->handle, 1, 0, 0, status ) == 3 ); + + /* Attempt to lock the cell for use by the current thread, using the + same sort of lock (read-only or read-write) as the parent object. + Report an error if this fails. */ + if( !dat1HandleLock( thisloc->handle, 2, 0, rdonly, status ) + && *status == SAI__OK ) { + *status = DAT__THREAD; + emsSetc( "C", cellname ); + emsSetc( "A", rdonly ? "read-only" : "read-write" ); + datMsg( "O", locator1 ); + emsRep( "","datCell: requested cell ^C within HDS object '^O' " + "cannot be locked for ^A access - another thread already has " + "a conflicting lock on the same component.", status ); + } thisloc->group_id = group_id; /* Secondary locator by definition */ diff --git a/datClen.c b/datClen.c index d8ab64f..8338eb9 100644 --- a/datClen.c +++ b/datClen.c @@ -116,7 +116,7 @@ datClen( const HDSLoc * locator, size_t * clen, int * status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datClen", 1, locator, 1, status ); if (locator->dataset_id <= 0) { *status = DAT__OBJIN; diff --git a/datClone.c b/datClone.c index 0e29428..55148e4 100644 --- a/datClone.c +++ b/datClone.c @@ -106,7 +106,7 @@ datClone(const HDSLoc *locator1, HDSLoc **locator2, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datClone", 1, locator1, 1, status ); clonedloc = dat1AllocLoc( status ); if (*status != SAI__OK) goto CLEANUP; diff --git a/datCoerc.c b/datCoerc.c index ca394a0..5785683 100644 --- a/datCoerc.c +++ b/datCoerc.c @@ -105,9 +105,6 @@ int datCoerc( const HDSLoc *locator1, int ndim, HDSLoc **locator2, int *status) if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); - *status = DAT__FATAL; emsRep("datCoerc", "datCoerc: Not yet implemented for HDF5", status); diff --git a/datConv.c b/datConv.c index 65cbb62..2bce65b 100644 --- a/datConv.c +++ b/datConv.c @@ -101,9 +101,6 @@ datConv(const HDSLoc *locator, const char *type_str, hdsbool_t *conv, *conv = 1; if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - *status = DAT__FATAL; emsRep("datConv", "datConv: Obsolete routine. Do not use", status); diff --git a/datCopy.c b/datCopy.c index cf00c58..45d9053 100644 --- a/datCopy.c +++ b/datCopy.c @@ -110,8 +110,8 @@ datCopy( const HDSLoc *locator1, const HDSLoc *locator2, if (*status != SAI__OK) return *status; /* Validate input locators. */ - dat1ValidateLocator( 1, locator1, status ); - dat1ValidateLocator( 1, locator2, status ); + dat1ValidateLocator( "datCopy", 1, locator1, 1, status ); + dat1ValidateLocator( "datCopy", 1, locator2, 0, status ); dau1CheckName( name_str, 1, cleanname, sizeof(cleanname), status ); if (*status != SAI__OK) return *status; diff --git a/datDrep.c b/datDrep.c index cdea37b..d2acc91 100644 --- a/datDrep.c +++ b/datDrep.c @@ -106,9 +106,6 @@ datDrep(const HDSLoc *locator, char **format_str, char **order_str, if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - *status = DAT__FATAL; emsRep("datDrep", "datDrep: Not yet implemented for HDF5", status); diff --git a/datErase.c b/datErase.c index 4d0ff9a..34825a9 100644 --- a/datErase.c +++ b/datErase.c @@ -98,7 +98,7 @@ datErase(const HDSLoc *locator, const char *name_str, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datErase", 1, locator, 0, status ); /* containing locator must refer to a group */ if (locator->group_id <= 0) { diff --git a/datFind.c b/datFind.c index 9084dc8..1319692 100644 --- a/datFind.c +++ b/datFind.c @@ -109,13 +109,14 @@ datFind( const HDSLoc *locator1, char cleanname[DAT__SZNAM+1]; HDSLoc * thisloc = NULL; H5O_info_t object_info; + int rdonly; int there = 0; int havegroup = 0; if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datFind", 1, locator1, 1, status ); if (*status != SAI__OK) return *status; /* containing locator must refer to a group */ @@ -205,20 +206,27 @@ datFind( const HDSLoc *locator1, } /* Store a pointer to the handle for the returned HDF object */ - thisloc->handle = dat1Handle( locator1, cleanname, status ); + thisloc->handle = dat1Handle( locator1, cleanname, 0, status ); /* We have to propagate groupness to the child */ if ( (locator1->grpname)[0] != '\0') hdsLink(thisloc, locator1->grpname, status); - /* Attempt to lock the component object for use by the current thread. - Report an error if this fails because it is already locked by another - thread. */ - if( !dat1HandleLock( thisloc->handle, 2, 0, status ) && *status == SAI__OK ) { + /* Determine if the current thread has a read-only or read-write lock + on the parent object, referenced by the supplied locator. */ + rdonly = ( dat1HandleLock( locator1->handle, 1, 0, 0, status ) == 3 ); + + /* Attempt to lock the component object for use by the current thread, + using the same sort of lock (read-only or read-write) as the + parent object. Report an error if this fails. */ + if( !dat1HandleLock( thisloc->handle, 2, 0, rdonly, status ) + && *status == SAI__OK ) { *status = DAT__THREAD; emsSetc( "C", name_str ); + emsSetc( "A", rdonly ? "read-only" : "read-write" ); datMsg( "O", locator1 ); emsRep( "","datFind: requested component ('^C') within HDS object '^O' " - "is locked by another thread.", status ); + "cannot be locked for ^A access - another thread already has " + "a conflicting lock on the same component.", status ); } if (*status != SAI__OK) goto CLEANUP; diff --git a/datGet.c b/datGet.c index 295607c..9d93737 100644 --- a/datGet.c +++ b/datGet.c @@ -128,7 +128,7 @@ datGet(const HDSLoc *locator, const char *type_str, int ndim, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datGet", 1, locator, 1, status ); /* For error messages */ datName( locator, namestr, status); diff --git a/datGet1C.c b/datGet1C.c index f5d9a93..700434e 100644 --- a/datGet1C.c +++ b/datGet1C.c @@ -146,7 +146,7 @@ datGet1C( const HDSLoc * locator, size_t maxval, size_t bufsize, char *buffer, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datGet1C", 1, locator, 1, status ); /* Verify that we have the correct number of values */ datSize( locator, actval, status ); diff --git a/datGetVC.c b/datGetVC.c index a111b1e..121507d 100644 --- a/datGetVC.c +++ b/datGetVC.c @@ -133,7 +133,7 @@ datGetVC( const HDSLoc * locator, size_t maxval, size_t bufsize, char *buffer, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datGetVC", 1, locator, 1, status ); datVec( locator, &vecLoc, status ); datGet1C( vecLoc, maxval, bufsize, buffer, pntrs, actval, status ); diff --git a/datIndex.c b/datIndex.c index cba1a90..b38af64 100644 --- a/datIndex.c +++ b/datIndex.c @@ -106,7 +106,7 @@ datIndex(const HDSLoc *locator1, int index, HDSLoc **locator2, int *status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datIndex", 1, locator1, 1, status ); datName( locator1, groupnam, status ); if (*status != SAI__OK) return *status; diff --git a/datLen.c b/datLen.c index a183e4c..60dc2f6 100644 --- a/datLen.c +++ b/datLen.c @@ -100,7 +100,7 @@ datLen( const HDSLoc * locator, size_t * clen, int * status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datLen", 1, locator, 1, status ); if (locator->dataset_id <= 0) { *status = DAT__OBJIN; diff --git a/datLock.c b/datLock.c index 96363e5..df6411a 100644 --- a/datLock.c +++ b/datLock.c @@ -13,7 +13,7 @@ * Library routine * Invocation: -* datLock( HDSLoc *locator, int recurs, int *status ); +* datLock( HDSLoc *locator, int recurs, int readonly, int *status ); * Arguments: * locator = HDSLoc * (Given) @@ -26,14 +26,33 @@ * a different thread - such components are left unchanged. This * operation is recursive - any children of the child components * are also locked, etc. +* readonly = int (Given) +* If non-zero, the object (and child objects if "recurs" is non-zero) +* is locked for read-only access. Otherwise it is locked for +* read-write access. * status = int* (Given and Returned) * Pointer to global status. * Description: -* This function locks an HDS object for exclusive use by the current -* thread. An error will then be reported if any other thread -* subsequently attempts to use the object for any purpose, whether -* through the supplied locator or any other locator. +* This function locks an HDS object for use by the current thread. +* An object can be locked for read-only access or read-write access. +* Multiple threads can lock an object simultaneously for read-only +* access, but only one thread can lock an object for read-write access +* at any one time. Use of any HDS function that may modify the object +* will fail with an error unless the thread has locked the object for +* read-write access. Use of an HDS function that cannot modify the +* object will fail with an error unless the thread has locked the +* object (in this case the lock can be either for read-only or +* read-write access). +* +* If "readonly" is zero (indicating the current thread wants to +* modify the object), this function will report an error if any +* other thread currently has a lock (read-only or read-write) on +* the object. +* +* If "readonly" is non-zero (indicating the current thread wants +* read-only access to the object), this function will report an error +* only if another thread currently has a read-write lock on the object. * * If the object is a structure, each component object will have its * own lock, which is independent of the lock on the parent object. A @@ -44,7 +63,12 @@ * * The current thread must unlock the object using datUnlock before it * can be locked for use by another thread. All objects are initially -* locked by the current thread when they are created. +* locked by the current thread when they are created. The type of +* access available to the object ("Read", "Write" or "Update") +* determines the type of the initial lock. For pre-existing objects, +* this is determined by the access mode specified when calling hdsOpen. +* For new and temporary objects, the initial lock is always a read-write +* lock. * Notes: * - An error will be reported if the supplied object is currently @@ -56,8 +80,9 @@ * thread. The exceptions are the functions that manage these locks - * datLock, datUnlock and datLocked. * - Attempting to lock an object that is already locked by the -* current thread has no effect. - +* current thread will change the type of lock (read-only or +* read-write) if the lock types differ, but will otherwise have no +* effect. * Authors: * DSB: David S Berry (DSB) @@ -115,13 +140,13 @@ #include "hds.h" #include "dat_err.h" -int datLock( HDSLoc *locator, int recurs, int *status ) { +int datLock( HDSLoc *locator, int recurs, int readonly, int *status ) { /* Check inherited status. */ if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 0, locator, status ); + dat1ValidateLocator( "datLock", 0, locator, 0, status ); /* Check we can de-reference "locator" safely. */ if( *status == SAI__OK ) { @@ -129,11 +154,12 @@ int datLock( HDSLoc *locator, int recurs, int *status ) { /* Attemp to lock the specified object, plus all its components. If the object could not be locked because it was already locked by another thread, report an error. */ - if( !dat1HandleLock( locator->handle, 2, recurs, status ) ) { + if( !dat1HandleLock( locator->handle, 2, recurs, readonly, status ) ) { if( *status == SAI__OK ) { *status = DAT__THREAD; + emsSetc( "U", readonly ? "read-only" : "read-write" ); datMsg( "O", locator ); - emsRep( " ", "datLock: Cannot lock HDS object '^O' for use by " + emsRep( " ", "datLock: Cannot lock HDS object '^O' for ^U use by " "the current thread:", status ); emsRep( " ", "It is already locked by another thread.", status ); } diff --git a/datLocked.c b/datLocked.c index 5837c73..7903001 100644 --- a/datLocked.c +++ b/datLocked.c @@ -26,23 +26,46 @@ * * 0: the supplied object is unlocked. This is the condition that must * be met for the current thread to be able to lock the supplied -* object using function datLock. This condition can be achieved -* by calling datUnlock. +* object for read-write access using function datLock. This condition +* can be achieved by releasing any existing locks using datUnlock. * -* 1: the supplied object is locked by the current thread. This is the -* condition that must be met for the current thread to be able to -* use the supplied object in any other HDS function (except for the -* locking and unlocking functions - see below). This condition can -* be achieved by calling datLock. +* 1: the supplied object is locked for reading and writing by the current +* thread. This is the condition that must be met for the current +* thread to be able to use the supplied object in any HDS function +* that might modify the object (except for the locking and unlocking +* functions - see below). This condition can be achieved by calling +* datLock. * -* 2: the supplied object is locked for use by a different thread. An -* error will be reported if the current thread attempts to use the -* object in any other HDS function. +* 2: the supplied object is locked for reading and writing by a different +* thread. An error will be reported if the current thread attempts to +* use the object in any other HDS function. +* +* 3: the supplied object is locked read-only by the current thread +* (and maybe other threads as well). This is the condition that must +* be met for the current thread to be able to use the supplied object +* in any HDS function that cannot modify the object. An error will be +* reported if the current thread attempts to use the object in any HDS +* function that could modify the object. This condition can be achieved +* by calling datLock. +* +* 4: the supplied object is not locked by the current thread, but is +* locked read-only by one or more other threads. An error will be +* reported if the current thread attempts to use the object in any +* other HDS function. * Description: * This function returns a value that indicates if the object -* specified by the supplied locator has been locked for exclusive -* use by a call to datLock. +* specified by the supplied locator has been locked for use by one or +* more threads. A thread can lock an object either for read-only +* access or for read-write access. The lock management functions +* (datLock and datUnlock) will ensure that any thread that requests +* and is given a read-write lock will have exclusive access to the +* object - no other locks of either type will be issued to other +* threads until the first thread releases the read-write lock using +* datUnlock. If a thread requests and is given a read-only lock, the +* lock management functions may issue read-only locks to other +* threads, but it will also ensure that no other thread is granted +* a read-write lock until all read-only locks have been released. * Notes: * - The locking performed by datLock, datUnlock and datLocked is @@ -117,12 +140,12 @@ int datLocked( const HDSLoc *locator, int *status ) { /* Validate input locator, but do not include the usual check that the object is locked by the current thread since we'll be performing that test as part of this function. */ - dat1ValidateLocator( 0, locator, status ); + dat1ValidateLocator( "datLocked", 0, locator, 1, status ); /* Get the value to return. Test status first so that we know it is safe to deference "locator". */ if( *status == SAI__OK ) { - result = dat1HandleLock( locator->handle, 1, 0, status ); + result = dat1HandleLock( locator->handle, 1, 0, 0, status ); } /* Return the result. */ diff --git a/datMap.c b/datMap.c index 9003156..26c2050 100644 --- a/datMap.c +++ b/datMap.c @@ -150,22 +150,6 @@ datMap(HDSLoc *locator, const char *type_str, const char *mode_str, int ndim, if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - - /* Get the HDF5 type code and confirm this is a primitive type */ - isprim = dau1CheckType( 1, type_str, &h5type, normtypestr, - sizeof(normtypestr), status ); - - if (!isprim) { - if (*status == SAI__OK) { - *status = DAT__TYPIN; - emsRepf("datMap_1", "datGet: Data type must be a primitive type and not '%s'", - status, normtypestr); - } - goto CLEANUP; - } - /* First have to validate the access mode */ switch (mode_str[0]) { case 'R': @@ -187,6 +171,22 @@ datMap(HDSLoc *locator, const char *type_str, const char *mode_str, int ndim, goto CLEANUP; } + /* Validate input locator. */ + dat1ValidateLocator( "datMap", 1, locator, (accmode & HDSMODE_READ), status ); + + /* Get the HDF5 type code and confirm this is a primitive type */ + isprim = dau1CheckType( 1, type_str, &h5type, normtypestr, + sizeof(normtypestr), status ); + + if (!isprim) { + if (*status == SAI__OK) { + *status = DAT__TYPIN; + emsRepf("datMap_1", "datGet: Data type must be a primitive type and not '%s'", + status, normtypestr); + } + goto CLEANUP; + } + /* Not allowed to map undefined data in READ or UPDATE mode */ if (accmode == HDSMODE_UPDATE || accmode == HDSMODE_READ) { hdsbool_t defined; diff --git a/datMapN.c b/datMapN.c index 51006c1..78b6209 100644 --- a/datMapN.c +++ b/datMapN.c @@ -111,9 +111,6 @@ int datMapN( HDSLoc* loc, const char * type, const char * mode, if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, loc, status ); - datShape( loc, ndim, dims, &actdim, status ); if (*status == SAI__OK) { diff --git a/datMould.c b/datMould.c index 9d8304b..a350995 100644 --- a/datMould.c +++ b/datMould.c @@ -100,7 +100,7 @@ datMould( HDSLoc *locator, int ndim, const hdsdim dims[], int *status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datMould", 1, locator, 0, status ); *status = DAT__FATAL; emsRep("datMould", "datMould: Not yet implemented for HDF5", diff --git a/datMove.c b/datMove.c index 5a14b2f..90b66cb 100644 --- a/datMove.c +++ b/datMove.c @@ -115,8 +115,9 @@ datMove( HDSLoc **locator1, const HDSLoc *locator2, const char *name_str, if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator2, status ); + /* Validate input locators. */ + dat1ValidateLocator( "datMove", 1, *locator1, 0, status ); + dat1ValidateLocator( "datMove", 1, locator2, 0, status ); dau1CheckName( name_str, 1, cleanname, sizeof(cleanname), status ); if (*status != SAI__OK) return *status; diff --git a/datName.c b/datName.c index 74260b8..6535d2a 100644 --- a/datName.c +++ b/datName.c @@ -113,7 +113,7 @@ datName(const HDSLoc *locator, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datName", 1, locator, 1, status ); objid = dat1RetrieveIdentifier( locator, status ); if (*status != SAI__OK) return *status; diff --git a/datNcomp.c b/datNcomp.c index 2dbfaa0..850ea19 100644 --- a/datNcomp.c +++ b/datNcomp.c @@ -99,7 +99,7 @@ datNcomp( const HDSLoc *locator, int *ncomp, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datNcomp", 1, locator, 1, status ); if (!dat1IsStructure( locator, status)) { *status = DAT__OBJIN; diff --git a/datNew.c b/datNew.c index 17e525f..a677b37 100644 --- a/datNew.c +++ b/datNew.c @@ -113,7 +113,7 @@ datNew( const HDSLoc *locator, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datNew", 1, locator, 0, status ); newloc = dat1New( locator, 0, name_str, type_str, ndim, dims, status ); diff --git a/datParen.c b/datParen.c index 0944cf0..6f134df 100644 --- a/datParen.c +++ b/datParen.c @@ -121,7 +121,7 @@ datParen( const HDSLoc *locator1, HDSLoc **locator2, int *status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datParen", 1, locator1, 1, status ); /* Need to get the relevant identfier */ objid = dat1RetrieveIdentifier( locator1, status ); diff --git a/datPrec.c b/datPrec.c index 50dce1d..1b9cdba 100644 --- a/datPrec.c +++ b/datPrec.c @@ -117,7 +117,7 @@ int datPrec( const HDSLoc * loc, size_t *nbytes, int *status ) { if ( *status != SAI__OK ) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, loc, status ); + dat1ValidateLocator( "datPrec", 1, loc, 1, status ); /* Get object type - assume this has to be nul-terminated */ datType( loc, type, status ); diff --git a/datPut.c b/datPut.c index 4f079cc..5c71916 100644 --- a/datPut.c +++ b/datPut.c @@ -128,7 +128,7 @@ datPut( const HDSLoc *locator, const char *type_str, int ndim, const hdsdim dims if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datPut", 1, locator, 0, status ); datName(locator, namestr, status); diff --git a/datPut1C.c b/datPut1C.c index 21725a4..68cb9f1 100644 --- a/datPut1C.c +++ b/datPut1C.c @@ -114,7 +114,7 @@ datPut1C( const HDSLoc * locator, size_t nval, const char *values[], int * statu if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datPut1C", 1, locator, 0, status ); /* Verify that we have the correct number of values */ datSize( locator, &actval, status ); diff --git a/datPutVC.c b/datPutVC.c index 3f57f1e..9c8d109 100644 --- a/datPutVC.c +++ b/datPutVC.c @@ -102,7 +102,7 @@ datPutVC( const HDSLoc * locator, size_t nval, const char *values[], int * statu if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datPutVC", 1, locator, 0, status ); /* Vectorize */ datVec( locator, &vecloc, status ); diff --git a/datRef.c b/datRef.c index 11d0016..8770888 100644 --- a/datRef.c +++ b/datRef.c @@ -132,7 +132,7 @@ int datRef( const HDSLoc * locator, char * ref, size_t reflen, int * status ) { if ( *status != SAI__OK ) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datRef", 1, locator, 1, status ); /* Obtain the data object path and container file name. */ diff --git a/datRefct.c b/datRefct.c index ad6220c..52686ea 100644 --- a/datRefct.c +++ b/datRefct.c @@ -104,7 +104,7 @@ datRefct(const HDSLoc *locator, int *refct, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datRefct", 1, locator, 1, status ); *refct = hds1PrimaryCount( locator->file_id, status ); return *status; } diff --git a/datRenam.c b/datRenam.c index 66272cf..b480398 100644 --- a/datRenam.c +++ b/datRenam.c @@ -103,7 +103,7 @@ datRenam( HDSLoc *locator, const char *name_str, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datRenam", 1, locator, 0, status ); /* We need to get the locator of the parent as we need to be moving it within the current group. */ diff --git a/datReset.c b/datReset.c index c1becfc..267c305 100644 --- a/datReset.c +++ b/datReset.c @@ -107,7 +107,7 @@ datReset(HDSLoc *locator, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datReset", 1, locator, 0, status ); datName( locator, name_str, status ); if (dat1IsStructure(locator, status)) { diff --git a/datRetyp.c b/datRetyp.c index edbba50..dc8a98d 100644 --- a/datRetyp.c +++ b/datRetyp.c @@ -96,7 +96,7 @@ datRetyp( const HDSLoc *locator, const char *type_str, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datRetyp", 1, locator, 0, status ); *status = DAT__FATAL; emsRep("datRetyp", "datRetyp: Not yet implemented for HDF5", diff --git a/datShape.c b/datShape.c index 1e4678a..ee940a0 100644 --- a/datShape.c +++ b/datShape.c @@ -105,7 +105,7 @@ datShape( const HDSLoc *locator, int maxdim, hdsdim dims[], if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datShape", 1, locator, 1, status ); /* Single cells should always be considered scalar. */ if( locator->iscell ){ diff --git a/datSize.c b/datSize.c index 2e3c620..e1419d4 100644 --- a/datSize.c +++ b/datSize.c @@ -102,7 +102,7 @@ datSize(const HDSLoc *locator, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datSize", 1, locator, 1, status ); /* Do not duplicate code from datShape -- just call it and we know then that it works for arrays of structures and for slices */ diff --git a/datSlice.c b/datSlice.c index e479f95..a67e1ef 100644 --- a/datSlice.c +++ b/datSlice.c @@ -137,7 +137,7 @@ datSlice(const HDSLoc *locator1, int ndim, const hdsdim lower[], if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datSlice", 1, locator1, 1, status ); /* We only work with primitives at the moment */ if (dat1IsStructure( locator1, status ) ) { diff --git a/datState.c b/datState.c index 3ade45b..c7a2647 100644 --- a/datState.c +++ b/datState.c @@ -99,7 +99,7 @@ datState( const HDSLoc *locator, hdsbool_t *state, int *status) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datState", 1, locator, 1, status ); if (dat1IsStructure(locator, status)) { *status = DAT__OBJIN; diff --git a/datTemp.c b/datTemp.c index efbfa73..f3248ba 100644 --- a/datTemp.c +++ b/datTemp.c @@ -159,8 +159,8 @@ datTemp( const char *type_str, int ndim, const hdsdim dims[], hdsNew(fname, "HDS_SCRATCH", "HDS_SCRATCH", 0, dims, &tmploc, status ); } - /* Lock the container file for use by the current thread. */ - datLock( tmploc, 0, status ); + /* Lock the container file for read-write access by the current thread. */ + datLock( tmploc, 0, 0, status ); /* Create a structure inside the temporary file. Compatibility with HDS suggests we call these TEMP_nnnn (although we only have to use the diff --git a/datThere.c b/datThere.c index c70db0c..a13cef9 100644 --- a/datThere.c +++ b/datThere.c @@ -101,7 +101,7 @@ datThere( const HDSLoc * locator, const char * name, hdsbool_t *there, if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datThere", 1, locator, 1, status ); /* containing locator must refer to a group */ if (locator->group_id <= 0) { diff --git a/datType.c b/datType.c index d70b761..fb31e73 100644 --- a/datType.c +++ b/datType.c @@ -99,7 +99,7 @@ datType( const HDSLoc *locator, char type_str[DAT__SZTYP+1], int * status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "datType", 1, locator, 1, status ); hdstyp = dat1Type( locator, status ); if (*status != SAI__OK) return *status; diff --git a/datUnlock.c b/datUnlock.c index f8121cb..4c4f4a6 100644 --- a/datUnlock.c +++ b/datUnlock.c @@ -30,23 +30,24 @@ * Pointer to global status. * Description: -* This function unlocks an HDS object. See datLock. +* This function ensures that the current thread does not have a lock +* of any type on the supplied HDS object. See datLock. * * The object must be locked again, using datLock, before it can be * used by any other HDS function. All objects are initially * locked by the current thread when they are created. * Notes: -* - An error will be reported if the supplied object is currently -* locked by another thread, but no error is reported if any component -* objects contained within the supplied object are locked by other -* threads (such objects are left unchanged). +* - No error is reported if the supplied object, or any child object, +* is current locked for read-only or read-write access by another thread. * - The majority of HDS functions will report an error if the object * supplied to the function has not been locked for use by the calling * thread. The exceptions are the functions that manage these locks - * datLock, datUnlock and datLocked. -* - Attempting to unlock an object that is already unlocked has no -* effect. +* - Attempting to unlock an object that is not locked by the current +* thread has no effect, and no error is reported. The datLocked +* function can be used to determine if the current thread has a lock +* on the object. * Authors: * DSB: David S Berry (DSB) @@ -111,28 +112,17 @@ int datUnlock( HDSLoc *locator, int recurs, int *status ) { -/* Local Variables; */ - int lstat; - /* Check inherited status. */ if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 0, locator, status ); + dat1ValidateLocator( "datUnlock", 0, locator, 0, status ); /* Check we can de-reference "locator" safely. */ if( *status == SAI__OK ) { -/* Attemp to unlock the specified object, plus all its components. If the - object could not be unlocked because it was already locked by another - thread, report an error. */ - lstat = dat1HandleLock( locator->handle, 3, recurs, status ); - if( *status == SAI__OK && lstat != 1 ) { - *status = DAT__THREAD; - datMsg( "O", locator ); - emsRep( " ", "datUnlock: Cannot unlock HDS object '^O':", status ); - emsRep( " ", "It is currently locked by another thread.", status ); - } +/* Attemp to unlock the specified object, plus all its components. */ + (void) dat1HandleLock( locator->handle, 3, recurs, 0, status ); } return *status; diff --git a/datUnmap.c b/datUnmap.c index 0246675..63f1a1c 100644 --- a/datUnmap.c +++ b/datUnmap.c @@ -113,6 +113,9 @@ datUnmap( HDSLoc * locator, int * status ) { /* if there is no mapped pointer in this locator do nothing */ if (!locator->regpntr) return *status; + /* Validate input locator. */ + dat1ValidateLocator( "datUnmap", 1, locator, (locator->accmode & HDSMODE_READ), status ); + /* We only copy back explicitly if we did not do a native mmap on the file */ if (!locator->uses_true_mmap) { @@ -141,9 +144,6 @@ datUnmap( HDSLoc * locator, int * status ) { if ( munmap( locator->pntr, locator->bytesmapped ) != 0 ) { if (*status == SAI__OK) { - - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); *status = DAT__FILMP; emsSyser( "MESSAGE", errno ); emsRep("datUnMap_4", "datUnmap: Error unmapping mapped memory: ^MESSAGE", status); diff --git a/datVec.c b/datVec.c index d1c3614..538fd86 100644 --- a/datVec.c +++ b/datVec.c @@ -121,7 +121,7 @@ datVec( const HDSLoc *locator1, HDSLoc **locator2, int *status ) { if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator1, status ); + dat1ValidateLocator( "datVec", 1, locator1, 1, status ); /* We cannot vectorise a discontiguous slice of an array. */ if ( locator1->isdiscont ) { diff --git a/dat_err.msg b/dat_err.msg index 88f4104..aefdb91 100644 --- a/dat_err.msg +++ b/dat_err.msg @@ -56,4 +56,5 @@ DTRNC HDF5E FILFM THREAD +MUTEX .END diff --git a/hds.h b/hds.h index b011fcf..b5213ca 100644 --- a/hds.h +++ b/hds.h @@ -361,7 +361,7 @@ datLen(const HDSLoc *locator, size_t *len, int *status); /*=========================================================*/ int -datLock( HDSLoc *locator, int recurs, int *status); +datLock( HDSLoc *locator, int recurs, int readonly, int *status); /*=======================================================================*/ diff --git a/hdsCopy.c b/hdsCopy.c index 75d6425..7a0e64a 100644 --- a/hdsCopy.c +++ b/hdsCopy.c @@ -105,9 +105,6 @@ hdsCopy( const HDSLoc *locator, const char *file_str, if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - *status = DAT__FATAL; emsRep("hdsCopy", "hdsCopy: Not yet implemented for HDF5", status); diff --git a/hdsFree.c b/hdsFree.c index 191f352..edd69ac 100644 --- a/hdsFree.c +++ b/hdsFree.c @@ -101,9 +101,6 @@ hdsFree(const HDSLoc *locator, int *status) { if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - /* It seems that HDSv4 flushes buffers as well as unlocking the file. This despite no hdsLock ever having been called */ H5Fflush( locator->file_id, H5F_SCOPE_LOCAL ); diff --git a/hdsGroup.c b/hdsGroup.c index 0050245..f1004df 100644 --- a/hdsGroup.c +++ b/hdsGroup.c @@ -100,7 +100,7 @@ hdsGroup(const HDSLoc *locator, char group_str[DAT__SZGRP+1], if (*status != SAI__OK) return *status; /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "hdsGroup", 1, locator, 1, status ); one_strlcpy( group_str, locator->grpname, DAT__SZGRP+1, status ); return *status; diff --git a/hdsLock.c b/hdsLock.c index 80ff282..2bc8a6a 100644 --- a/hdsLock.c +++ b/hdsLock.c @@ -99,9 +99,6 @@ hdsLock(const HDSLoc *locator, int *status) { if (*status != SAI__OK) return *status; - /* Validate input locator. */ - dat1ValidateLocator( 1, locator, status ); - *status = DAT__FATAL; emsRep("hdsLock", "hdsLock: Not yet implemented for HDF5", status); diff --git a/hdsNew.c b/hdsNew.c index 9910645..c94690b 100644 --- a/hdsNew.c +++ b/hdsNew.c @@ -181,8 +181,9 @@ hdsNew(const char *file_str, if (*status == SAI__OK) file_id = 0; /* handed file to locator */ /* Create a new Handle structure describing the new object and store - it in the locator. */ - tmploc->handle = dat1Handle( NULL, fname, status ); + it in the locator. Lock it for read-write access by the current + thread. */ + tmploc->handle = dat1Handle( NULL, fname, 0, status ); /* We use dat1New instead of datNew so that we do not have to follow up immediately with a datFind */ diff --git a/hdsOpen.c b/hdsOpen.c index 02f1727..0b42ff8 100644 --- a/hdsOpen.c +++ b/hdsOpen.c @@ -119,6 +119,7 @@ hdsOpen( const char *file_str, const char *mode_str, hid_t group_id = 0; htri_t filstat = 0; unsigned int flags = 0; + int rdonly = 0; *locator = NULL; if (*status != SAI__OK) return *status; @@ -138,6 +139,7 @@ hdsOpen( const char *file_str, const char *mode_str, case 'R': case 'r': flags = H5F_ACC_RDONLY; + rdonly = 1; break; default: *status = DAT__MODIN; @@ -225,7 +227,7 @@ hdsOpen( const char *file_str, const char *mode_str, create a new handle structure for the top level data object in the file. Store the handle pointer in the locator. */ if( *locator ) { - if( !handle ) handle = dat1Handle( NULL, fname, status ); + if( !handle ) handle = dat1Handle( NULL, fname, rdonly, status ); (*locator)->handle = handle; } diff --git a/hdsTest.c b/hdsTest.c index 577af29..b86f844 100644 --- a/hdsTest.c +++ b/hdsTest.c @@ -85,6 +85,8 @@ static void *test3ThreadSafety( void *data ); typedef struct threadData { HDSLoc *loc; int failed; + int rdonly; + int id; } threadData; int main (void) { @@ -1045,9 +1047,10 @@ static void testThreadSafety( const char *path, int *status ) { ival ); } -/* Check the top level object is locked by the current thread. */ +/* Check the top level object is locked for read-only access by the current + thread. */ ival = datLocked( loc1, status ); - if( ival != 1 && *status == SAI__OK ) { + if( ival != 3 && *status == SAI__OK ) { *status = DAT__FATAL; emsRep( "", "testThreadSafety error 101: Top-level object not " "locked by current thread.", status ); @@ -1055,7 +1058,7 @@ static void testThreadSafety( const char *path, int *status ) { /* Check the bottom level object is also locked by the current thread. */ ival = datLocked( loc4, status ); - if( ival != 1 && *status == SAI__OK ) { + if( ival != 3 && *status == SAI__OK ) { *status = DAT__FATAL; emsRep( "", "testThreadSafety error 102: Bottom-level object not " "locked by current thread.", status ); @@ -1084,7 +1087,7 @@ static void testThreadSafety( const char *path, int *status ) { /* Check the top level object is locked by the current thread. */ ival = datLocked( loc1b, status ); - if( ival != 1 && *status == SAI__OK ) { + if( ival != 3 && *status == SAI__OK ) { *status = DAT__FATAL; emsRep( "", "testThreadSafety error 201: Top-level object not " "locked by current thread.", status ); @@ -1092,25 +1095,39 @@ static void testThreadSafety( const char *path, int *status ) { /* Check the bottom level object is also locked by the current thread. */ ival = datLocked( loc4b, status ); - if( ival != 1 && *status == SAI__OK ) { + if( ival != 3 && *status == SAI__OK ) { *status = DAT__FATAL; emsRep( "", "testThreadSafety error 202: Bottom-level object not " "locked by current thread.", status ); } +/* Promote the lock to a read/write lock using the first locator. */ + datLock( loc1, 1, 0, status ); + +/* Check the other locator now also has a read/write lock. */ + ival = datLocked( loc1b, status ); + if( ival != 1 && *status == SAI__OK ) { + *status = DAT__FATAL; + emsRep( "", "testThreadSafety error 2021: Top-level object not " + "locked by current thread.", status ); + } + /* Required for use of EMS within threads. */ emsMark(); /* Create two threads, and pass a locator for the top-level object to each. - Note, these locators are still locked by the current thread, so we - should get DAT__THREAD errors when test1ThreadSafety tries to use them. */ - pthread_create( &t1, NULL, test1ThreadSafety, loc1 ); - pthread_create( &t2, NULL, test1ThreadSafety, loc1b ); + Note, these locators are still locked for read/write by the current thread, + so we should get DAT__THREAD errors when test1ThreadSafety tries to use + them. */ + if( *status == SAI__OK ) { + pthread_create( &t1, NULL, test1ThreadSafety, loc1 ); + pthread_create( &t2, NULL, test1ThreadSafety, loc1b ); /* Wait for them to terminate. */ - pthread_join( t1, NULL ); - pthread_join( t2, NULL ); - emsStat( status ); + pthread_join( t1, NULL ); + pthread_join( t2, NULL ); + emsStat( status ); + } /* Unlock the top level object using the first locator. Then check that it is also unlocked using the second locator. */ @@ -1138,83 +1155,140 @@ static void testThreadSafety( const char *path, int *status ) { "locked.", status ); } - /* Attempt to access the two top-level objects in two separate threads. - Each thread attempt to lock the object, but only one can win. The other - should report an error. */ - threaddata1.loc = loc1; - pthread_create( &t1, NULL, test2ThreadSafety, &threaddata1 ); - threaddata2.loc = loc1b; - pthread_create( &t2, NULL, test2ThreadSafety, &threaddata2 ); + Each thread attempt to lock the object read/write, but only one can win. + The other should report an error. */ + if( *status == SAI__OK ) { + threaddata1.id = 1; + threaddata1.rdonly = 0; + threaddata1.loc = loc1; + pthread_create( &t1, NULL, test2ThreadSafety, &threaddata1 ); + threaddata2.id = 2; + threaddata2.rdonly = 0; + threaddata2.loc = loc1b; + pthread_create( &t2, NULL, test2ThreadSafety, &threaddata2 ); /* Wait for them to terminate. */ - pthread_join( t1, NULL ); - pthread_join( t2, NULL ); - emsStat( status ); + pthread_join( t1, NULL ); + pthread_join( t2, NULL ); + emsStat( status ); /* Check one, and only one, failed. */ - if( threaddata1.failed + threaddata2.failed != 1 && *status == SAI__OK ) { - *status = DAT__FATAL; - emsRepf( "", "testThreadSafety error 205: %d lock attempts succeeded " - "- expected 1 to succeed.", status, - threaddata1.failed + threaddata2.failed ); + if( threaddata1.failed + threaddata2.failed != 1 && *status == SAI__OK ) { + *status = DAT__FATAL; + emsRepf( "", "testThreadSafety error 205: %d read-write lock attempts " + "succeeded - expected 1 to succeed.", status, + threaddata1.failed + threaddata2.failed ); + } } -/* Lock both top level locators for use by the current thread. The second - of these calls will have no effect as the two locators refer to the same - object. These locks are recursive. */ - datLock( loc1, 1, status ); - datLock( loc1b, 1, status ); - -/* Annul the first primary locator for the file. */ - datAnnul( &loc1, status ); +/* Attempt to access the two top-level objects in two separate threads. + Each thread attempt to lock the object read-only. Both should be + successful. */ + if( *status == SAI__OK ) { + threaddata1.rdonly = 1; + threaddata1.loc = loc1; + pthread_create( &t1, NULL, test2ThreadSafety, &threaddata1 ); + threaddata2.rdonly = 1; + threaddata2.loc = loc1b; + pthread_create( &t2, NULL, test2ThreadSafety, &threaddata2 ); +/* Wait for them to terminate. */ + pthread_join( t1, NULL ); + pthread_join( t2, NULL ); + emsStat( status ); +/* Check neither failed. */ + if( threaddata1.failed + threaddata2.failed > 0 && *status == SAI__OK ) { + *status = DAT__FATAL; + emsRepf( "", "testThreadSafety error 2051: %d read-only lock attempts failed " + "- expected 0 to fail.", status, + threaddata1.failed + threaddata2.failed ); + } + } +/* Lock both top level locators for read-only use by the current thread. The + second of these calls will have no effect as the two locators refer to the + same object. These locks are recursive. */ + datLock( loc1, 1, 1, status ); + datLock( loc1b, 1, 1, status ); +/* Annul the first primary locator for the file. */ + datAnnul( &loc1, status ); /* Each thread creates a temporary object holding a large array of doubles and does some heavy work on it. */ - pthread_create( &t1, NULL, test3ThreadSafety, &threaddata1 ); - pthread_create( &t2, NULL, test3ThreadSafety, &threaddata2 ); + if( *status == SAI__OK ) { + pthread_create( &t1, NULL, test3ThreadSafety, &threaddata1 ); + pthread_create( &t2, NULL, test3ThreadSafety, &threaddata2 ); /* Wait for them to terminate. */ - pthread_join( t1, NULL ); - pthread_join( t2, NULL ); - emsStat( status ); + pthread_join( t1, NULL ); + pthread_join( t2, NULL ); + emsStat( status ); -/* Unlock the locators for the arrays so that the current thread can - access them. */ - datLock( threaddata1.loc, 0, status ); - datLock( threaddata2.loc, 0, status ); +/* Lock the locators for the arrays so that the current thread can have + read-only access them. */ + datLock( threaddata1.loc, 0, 1, status ); + datLock( threaddata2.loc, 0, 1, status ); /* Check the two threads created equal values. */ - dim = 10000; - datMap( threaddata1.loc, "_DOUBLE", "Read", 1, &dim, (void **) &ip1, status ); - datMap( threaddata2.loc, "_DOUBLE", "Read", 1, &dim, (void **) &ip2, status ); - if( *status == SAI__OK ) { - for( i = 0; i < dim; i++ ) { - if( ip1[i] != ip2[i] ) { - *status = DAT__FATAL; - emsRepf( "", "testThreadSafety error 206: Threads created " - "different values (%.20g anbd %.20g) at element %d", - status, ip1[i], ip2[i], i ); + dim = 10000; + datMap( threaddata1.loc, "_DOUBLE", "Read", 1, &dim, (void **) &ip1, status ); + datMap( threaddata2.loc, "_DOUBLE", "Read", 1, &dim, (void **) &ip2, status ); + if( *status == SAI__OK ) { + for( i = 0; i < dim; i++ ) { + if( ip1[i] != ip2[i] ) { + *status = DAT__FATAL; + emsRepf( "", "testThreadSafety error 206: Threads created " + "different values (%.20g anbd %.20g) at element %d", + status, ip1[i], ip2[i], i ); + } } } - } - - datAnnul( &(threaddata1.loc), status ); - datAnnul( &(threaddata2.loc), status ); - - - - + datUnmap( threaddata1.loc, status ); + datUnmap( threaddata2.loc, status ); + +/* Attempt to modify each object. This should generate an error since + the current thread does not have a read-write lock on either of them. */ + dims[0] = 10; + datCell( threaddata1.loc, 1, dims, &loc3, status ); + if( *status == SAI__OK ) { + datPut0D( loc3, 1.0, status ); + if( *status == DAT__THREAD ) { + emsAnnul( status ); + } else { + int oldstat = *status; + emsAnnul( status ); + *status = DAT__FATAL; + emsRepf("", "testThreadSafety error 207: Expected a DAT__LOCIN " + "error but got status=%d", status, oldstat ); + } + datAnnul( &loc3, status ); + } + datCell( threaddata2.loc, 1, dims, &loc3, status ); + if( *status == SAI__OK ) { + datPut0D( loc3, 1.0, status ); + if( *status == DAT__THREAD ) { + emsAnnul( status ); + } else { + int oldstat = *status; + emsAnnul( status ); + *status = DAT__FATAL; + emsRepf("", "testThreadSafety error 208: Expected a DAT__LOCIN " + "error but got status=%d", status, oldstat ); + } + datAnnul( &loc3, status ); + } + datAnnul( &(threaddata1.loc), status ); + datAnnul( &(threaddata2.loc), status ); + } /* Check for the exit status */ - emsStat( status ); + if( *status == SAI__OK ) emsStat( status ); emsRlse(); @@ -1275,10 +1349,16 @@ void *test1ThreadSafety( void *data ) { if( status == DAT__THREAD ) { emsAnnul( &status ); + + } else if( status == SAI__OK ) { + status = DAT__FATAL; + emsRepf("", "testThreadSafety error A1: Expected a DAT__THREAD " + "error but no error was reported", &status ); + } else { int oldstat = status; emsAnnul( &status ); - status = DAT__THREAD; + status = DAT__FATAL; emsRepf("", "testThreadSafety error A1: Expected a DAT__THREAD " "error but got status=%d", &status, oldstat ); } @@ -1292,8 +1372,9 @@ void *test2ThreadSafety( void *data ) { HDSLoc *loc1 = tdata->loc; HDSLoc *loc2 = NULL; int status = SAI__OK; + int expect = tdata->rdonly ? 3 : 1; - datLock( loc1, 1, &status ); + datLock( loc1, 1, tdata->rdonly, &status ); if( status == DAT__THREAD ) { emsAnnul( &status ); tdata->failed = 1; @@ -1302,7 +1383,7 @@ void *test2ThreadSafety( void *data ) { datFind( loc1, "Records", &loc2, &status ); /* Check the component locator is locked by the current thread. */ - if( datLocked( loc2, &status ) != 1 && status == SAI__OK ) { + if( datLocked( loc2, &status ) != expect && status == SAI__OK ) { status = DAT__FATAL; emsRepf("", "testThreadSafety error B1: loc2 is not locked by " "current thread.", &status ); diff --git a/hdsgroups.c b/hdsgroups.c index 3843918..2fea808 100644 --- a/hdsgroups.c +++ b/hdsgroups.c @@ -292,7 +292,7 @@ static int hds2Link(HDSLoc *locator, const char *group_str, int *status) { if (*status != SAI__OK) return *status; /* Validate supplied locator */ - dat1ValidateLocator( 1, locator, status ); + dat1ValidateLocator( "hdsLink", 1, locator, 0, status ); /* If we get a zero length string this either means we are trying to unlink the locator from the group or we have a bug in the calling code or else