Skip to content

Commit

Permalink
CORE-589: In Core Crypto, Address TSAN #3
Browse files Browse the repository at this point in the history 
The FileService holds an RLP Coder and a file service is now called via multiple threads.  Hence TSAN issues have arisen.
  • Loading branch information
Ed Gamble committed Sep 23, 2019
1 parent 2dfa99d commit 3087817
Showing 1 changed file with 36 additions and 113 deletions.
149 changes: 36 additions & 113 deletions ethereum/rlp/BRRlpCoder.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
#include "ethereum/util/BRUtil.h"
#include "BRRlpCoder.h"

static void
rlpCoderReclaimInternal (BRRlpCoder coder) ;

static int
rlpDecodeStringEmptyCheck (BRRlpCoder coder, BRRlpItem item);

Expand Down Expand Up @@ -125,21 +122,8 @@ rlpCoderCreate (void) {
return coder;
}

extern void
rlpCoderRelease (BRRlpCoder coder) {
pthread_mutex_lock(&coder->lock);

// Every single Item must be returned!
assert (NULL == coder->busy);
rlpCoderReclaimInternal (coder);

pthread_mutex_unlock(&coder->lock);
pthread_mutex_destroy(&coder->lock);
free (coder);
}

static void
rlpCoderReclaimInternal (BRRlpCoder coder) {
_rlpCoderReclaimInternal (BRRlpCoder coder) {
BRRlpItem item = coder->free;
while (item != NULL) {
BRRlpItem next = item->next; // save 'next' before release...
Expand All @@ -153,22 +137,26 @@ rlpCoderReclaimInternal (BRRlpCoder coder) {
extern void
rlpCoderReclaim (BRRlpCoder coder) {
pthread_mutex_lock(&coder->lock);
rlpCoderReclaimInternal (coder);
_rlpCoderReclaimInternal (coder);
pthread_mutex_unlock(&coder->lock);
}

extern int
rlpCoderBusyCount (BRRlpCoder coder) {
int count = 0;
for (BRRlpItem found = coder->busy; NULL != found; found = found->next)
count++;
return count;
extern void
rlpCoderRelease (BRRlpCoder coder) {
pthread_mutex_lock(&coder->lock);

// Every single Item must be returned!
assert (NULL == coder->busy);
_rlpCoderReclaimInternal (coder);

pthread_mutex_unlock(&coder->lock);
pthread_mutex_destroy(&coder->lock);
free (coder);
}

static BRRlpItem
rlpCoderAcquireItem (BRRlpCoder coder) {
_rlpCoderAcquireItemInternal (BRRlpCoder coder) {
BRRlpItem item = NULL;
pthread_mutex_lock(&coder->lock);

// Get `item` from `coder->free` or `calloc`
if (NULL != coder->free) {
Expand All @@ -188,13 +176,19 @@ rlpCoderAcquireItem (BRRlpCoder coder) {
// Update `coder` to show `item` as busy.
coder->busy = item;

return item;
}

static BRRlpItem
rlpCoderAcquireItem (BRRlpCoder coder) {
pthread_mutex_lock(&coder->lock);
BRRlpItem item = _rlpCoderAcquireItemInternal (coder);
pthread_mutex_unlock(&coder->lock);
return item;
}

static void
rlpCoderReturnItem (BRRlpCoder coder, BRRlpItem prev, BRRlpItem item, BRRlpItem next) {
pthread_mutex_lock(&coder->lock);
_rlpCoderReturnItemInternal (BRRlpCoder coder, BRRlpItem prev, BRRlpItem item, BRRlpItem next) {
assert (NULL == item->next && NULL == item->prev &&
0 == item->bytesCount && 0 == item->itemsCount);

Expand All @@ -215,17 +209,26 @@ rlpCoderReturnItem (BRRlpCoder coder, BRRlpItem prev, BRRlpItem item, BRRlpItem

// Update `coder` to show `item` as free.
coder->free = item;

pthread_mutex_unlock(&coder->lock);
}

static void
itemRelease (BRRlpCoder coder, BRRlpItem item) {
_rlpCoderReleaseItemInternal (BRRlpCoder coder, BRRlpItem item) {
for (size_t index = 0; index < item->itemsCount; index++)
_rlpCoderReleaseItemInternal (coder, item->items[index]);

// Surely get these before itemReleaseMemory() blows them away.
BRRlpItem prev = item->prev;
BRRlpItem next = item->next;

itemReleaseMemory(item);
rlpCoderReturnItem(coder, prev, item, next);
_rlpCoderReturnItemInternal (coder, prev, item, next);
}

static void
rlpCoderReleaseItem (BRRlpCoder coder, BRRlpItem item) {
pthread_mutex_lock(&coder->lock);
_rlpCoderReleaseItemInternal (coder, item);
pthread_mutex_unlock(&coder->lock);
}

static int
Expand Down Expand Up @@ -277,84 +280,6 @@ rlpCoderHasFailed (BRRlpCoder coder) {
return coder->failed;
}

#if 0
static BRRlpItem
itemCreate (BRRlpCoder coder,
uint8_t *bytes, size_t bytesCount, int takeBytes) {
BRRlpItem item = calloc (1, sizeof (struct BRRlpItemRecord));

item->type = CODER_ITEM;
item->bytesCount = bytesCount;
if (takeBytes)
item->bytes = bytes;
else {
item->bytes = (item->bytesCount > ITEM_DEFAULT_BYTES_COUNT
? malloc (item->bytesCount)
: item->bytesArray);
memcpy (item->bytes, bytes, item->bytesCount);
}
item->itemsCount = 0;
item->items = NULL;

return item;
}

static BRRlpItem
itemCreateList (BRRlpCoder coder,
uint8_t *bytes, size_t bytesCount, int takeBytes,
BRRlpItem *items, size_t itemsCount) {
BRRlpItem item = itemCreate(coder, bytes, bytesCount, takeBytes);
item->type = CODER_LIST;
item->itemsCount = itemsCount;
item->items = (item->itemsCount > ITEM_DEFAULT_ITEMS_COUNT
? calloc (item->itemsCount, sizeof (BRRlpItem))
: item->itemsArray);
for (int i = 0; i < itemsCount; i++)
item->items[i] = items[i];

return item;
}
#endif

/**
* Return a new BRRlpContext by appending the two provided contexts. Both provided contexts
* must be for CODER_ITEM (othewise an 'assert' is raised); the appending is performed by simply
* concatenating the two context's byte arrays.
*
* If release is TRUE, then both the provided contexts are released; thereby freeing their memory.
*
*/
#if 0
static BRRlpItem
itemCreateAppend (BRRlpCoder coder, BRRlpItem context1, BRRlpItem context2, int release) {
assert (CODER_ITEM == context1->type && CODER_ITEM == context2->type);

BRRlpItem item = calloc (1, sizeof (struct BRRlpItemRecord));

item->type = CODER_ITEM;

item->bytesCount = context1->bytesCount + context2->bytesCount;
item->bytes = (item->bytesCount > ITEM_DEFAULT_BYTES_COUNT
? malloc (item->bytesCount)
: item->bytesArray);

memcpy (&item->bytes[0], context1->bytes, context1->bytesCount);
memcpy (&item->bytes[context1->bytesCount], context2->bytes, context2->bytesCount);

item->itemsCount = 0;
item->items = NULL;

if (release) {
// assert (context2.bytes != context1.bytes || context2.bytes == NULL || context1.bytes == NULL);
// assert (context2.items != context1.items || context2.items == NULL || context1.items == NULL);
itemRelease(coder, context1);
itemRelease(coder, context2);
}

return item;
}
#endif

// The largest number supported for encoding is a UInt256 - which is representable as 32 bytes.
#define CODER_NUMBER_BYTES_LIMIT (256/8)

Expand Down Expand Up @@ -1071,9 +996,7 @@ rlpShowItemInternal (BRRlpCoder coder, BRRlpItem context, const char *topic, int
extern void
rlpReleaseItem (BRRlpCoder coder, BRRlpItem item) {
assert (itemIsValid(coder, item));
for (size_t index = 0; index < item->itemsCount; index++)
rlpReleaseItem(coder, item->items[index]);
itemRelease(coder, item);
rlpCoderReleaseItem (coder, item);
}

extern void
Expand Down

0 comments on commit 3087817

Please sign in to comment.