refactor: move locking responsibility to user

Makefile

@@ -1,11 +1,11 @@
 # =====CONFIG=====
 BUILD_DIR := ./build
-SRC_DIR := ./src
+SRC_DIRS := ./src ./trie/src
-INCLUDE_DIR := ./include
+INCLUDE_DIRS := ./trie/include
 TEST_DIR := test
 CORES != nproc
 
-SRCS := $(shell find '$(SRC_DIR)' '$(INCLUDE_DIR)' \( -iname '*.cpp' -or -iname '*.c' -or -iname '*.h' \))
+SRCS := $(shell find $(SRC_DIRS) $(INCLUDE_DIRS) \( -iname '*.cpp' -or -iname '*.c' -or -iname '*.h' \))
 
 
 # =====RECIPES=====

src/main.cpp

@@ -34,7 +34,12 @@ static const std::string index_page = R"(
 crow::response add_redirect(std::string base_url, Trie *trie, const char *url,
                             bool secure) {
   Entry *new_entry = entry_new(Redirect, url);
+
+  // The key already gets copied into the trie, so this pointer is safe to use
+  // ever after unlocking the trie
+  trie_wlock(trie);
   char *key = trie_add_random(trie, new_entry, secure);
+  trie_unlock(trie);
 
   if (key == NULL) {
     return crow::response(crow::status::INTERNAL_SERVER_ERROR);
@@ -64,7 +69,10 @@ bool store_paste(const char *key, const char *body) {
 crow::response add_paste(std::string base_url, Trie *trie, const char *body,
                          bool secure) {
   Entry *new_entry = entry_new(Paste, "");
+
+  trie_wlock(trie);
   char *key = trie_add_random(trie, new_entry, secure);
+  trie_unlock(trie);
 
   if (key == NULL) {
     return crow::response(crow::status::INTERNAL_SERVER_ERROR);
@@ -95,6 +103,7 @@ int main() {
   std::cout << "Populating trie from file '" << file_path << "'..."
             << std::endl;
 
+  // Web server hasn't started yet, so there's no point in locking the trie
   int count = trie_populate(trie, file_path.c_str());
 
   if (count == -1) {
@@ -102,7 +111,8 @@ int main() {
 
     exit(1);
   } else {
-    std::cout << "Added " << count << " (" << trie_size(trie) <<  ") entries to trie." << std::endl;
+    std::cout << "Added " << count << " (" << trie_size(trie)
+              << ") entries to trie." << std::endl;
   }
 
   // Create pastes directory if not present
@@ -119,6 +129,7 @@ int main() {
   CROW_ROUTE(app, "/<string>")
       .methods(crow::HTTPMethod::Get)(
           [trie](crow::response &res, std::string key) {
+            trie_rlock(trie);
             Entry *entry = trie_search(trie, key.c_str());
 
             if (entry != NULL) {
@@ -132,6 +143,7 @@ int main() {
             }
 
             res.end();
+            trie_unlock(trie);
           });
 
   // Add a new Redirect with a short randomly generated key
@@ -160,7 +172,9 @@ int main() {
 
             Entry *new_entry = entry_new(Redirect, req.body.c_str());
 
+            trie_wlock(trie);
             bool added = trie_add(trie, key.c_str(), new_entry);
+            trie_unlock(trie);
 
             if (!added) {
               return crow::response(crow::status::CONFLICT);
@@ -194,7 +208,9 @@ int main() {
             AUTH();
 
             Entry *new_entry = entry_new(Paste, "");
+            trie_wlock(trie);
             bool added = trie_add(trie, key.c_str(), new_entry);
+            trie_unlock(trie);
 
             if (!added) {
               return crow::response(crow::status::CONFLICT);

trie/README.md

@@ -0,0 +1,4 @@
+# Trie design
+
+This trie is a modified version of the custom trie I had to implement for my
+Algorithms & Datastructures 3 course.

trie/include/trie.h

@@ -94,7 +94,8 @@ bool trie_add_no_lock(Trie *trie, const char *key, Entry *entry);
  * @param trie
  * @param entry entry to add
  * @param secure whether to generate a longer, more secure random key
- * @return the generated key
+ * @return pointer to the generated key. This pointer is safe to use after
+ * unlocking the trie, and should be freed manually.
  */
 char *trie_add_random(Trie *trie, Entry *entry, bool secure);
 
@@ -116,4 +117,28 @@ bool trie_remove(Trie *trie, const char *key);
  */
 size_t trie_size(Trie *trie);
 
+/*
+ * Acquire a read lock on the trie.
+ *
+ * @return 0 if successful, non-zero otherwise (return value of
+ * pthread_rwlock_rdlock)
+ */
+int trie_rlock(Trie *trie);
+
+/*
+ * Acquire a write lock on the trie.
+ *
+ * @return 0 if successful, non-zero otherwise (return value of
+ * pthread_rwlock_wrlock)
+ */
+int trie_wlock(Trie *trie);
+
+/*
+ * Release the lock on a trie after having acquired it beforehand.
+ *
+ * @return 0 if successful, non-zero otherwise (return value of
+ * pthread_rwlock_unlock)
+ */
+int trie_unlock(Trie *trie);
+
 #endif // AD3_TERNARYTRIE

trie/src/trie.c

@@ -5,6 +5,7 @@
 #include <string.h>
 
 #include "trie.h"
+#include "trie_entry.c"
 #include "trie_node.c"
 
 typedef struct ttrie {
@@ -39,41 +40,6 @@ void trie_free(Trie *trie) {
 
 bool trie_add_no_lock(Trie *trie, const char *key, Entry *entry);
 
-EntryType entry_type_from_char(char c) {
-  switch (c) {
-  case '0':
-    return Redirect;
-  case '1':
-    return Paste;
-  default:
-    return Unknown;
-  }
-}
-
-char entry_type_to_char(EntryType et) {
-  switch (et) {
-  case Redirect:
-    return '0';
-  case Paste:
-    return '1';
-  default:
-    return '\0';
-  }
-}
-
-Entry *entry_new(EntryType type, const char *string) {
-  Entry *entry = malloc(sizeof(Entry));
-  entry->type = type;
-
-  if (string != NULL) {
-    entry->string = strdup(string);
-  } else {
-    entry->string = NULL;
-  }
-
-  return entry;
-}
-
 int trie_populate(Trie *trie, const char *file_path) {
   trie->file_path = strdup(file_path);
 
@@ -156,28 +122,6 @@ SearchResult trie_search_node(Trie *trie, const char *key) {
 
     i += (*child_ptr)->string_len;
 
-/*     offset = 0; */
-
-/*     // We iterate over each character on the edge and compare it to the string. */
-/*     while (offset < (*child_ptr)->string_len) { */
-/*       // Our string ends in the middle of an edge, so it's definitely not in */
-/*       // the trie. */
-/*       if (key[i + offset] == DELIMITER) { */
-/*         return out; */
-/*       } */
-
-/*       // We compare each character with the characters in the skipped */
-/*       // substring. If they don't match, we know the string isn't in the */
-/*       // trie. */
-/*       if (key[i + offset] != ((*child_ptr)->string[offset])) { */
-/*         return out; */
-/*       } */
-
-/*       offset++; */
-/*     } */
-
-/*     i += offset; */
-
     if (key[i] != DELIMITER) {
       node_ptr = child_ptr;
     }
@@ -202,8 +146,6 @@ SearchResult trie_search_node(Trie *trie, const char *key) {
  * @return true if the string is present in the trie, false otherwise
  */
 Entry *trie_search(Trie *trie, const char *key) {
-  pthread_rwlock_rdlock(&trie->lock);
-
   SearchResult res = trie_search_node(trie, key);
 
   Entry *return_value = NULL;
@@ -212,8 +154,6 @@ Entry *trie_search(Trie *trie, const char *key) {
     return_value = res.child->entry;
   }
 
-  pthread_rwlock_unlock(&trie->lock);
-
   return return_value;
 }
 
@@ -243,8 +183,7 @@ bool trie_add_no_lock(Trie *trie, const char *string, Entry *entry) {
     if (*child_node_ptr == NULL) {
       child_node = tnode_init();
 
-      while (offset < TRIE_MAX_SKIP_SIZE &&
+      while (offset < TRIE_MAX_SKIP_SIZE && string[i + offset] != DELIMITER) {
-             string[i + offset] != DELIMITER) {
         offset++;
       }
 
@@ -327,9 +266,7 @@ bool trie_add_no_lock(Trie *trie, const char *string, Entry *entry) {
   return true;
 }
 
-bool trie_add_persistent(Trie *trie, const char *key, Entry *entry) {
+bool trie_add(Trie *trie, const char *key, Entry *entry) {
-  bool return_value = false;
-
   if (trie->file_path != NULL) {
     // Easiest way to make sure we don't add duplicate entries
     // We use an internal function that doesn't require a read lock, as we're
@@ -350,8 +287,6 @@ bool trie_add_persistent(Trie *trie, const char *key, Entry *entry) {
     fputs(" ", fp);
     fputs(entry->string, fp);
     fputs("\n", fp);
-
-    fclose(fp);
   }
 
   // This function *should* always return true. Otherwise, the function would've
@@ -359,19 +294,7 @@ bool trie_add_persistent(Trie *trie, const char *key, Entry *entry) {
   return trie_add_no_lock(trie, key, entry);
 }
 
-bool trie_add(Trie *trie, const char *key, Entry *entry) {
-  pthread_rwlock_wrlock(&trie->lock);
-
-  bool return_value = trie_add_persistent(trie, key, entry);
-
-  pthread_rwlock_unlock(&trie->lock);
-
-  return return_value;
-}
-
 char *trie_add_random(Trie *trie, Entry *entry, bool secure) {
-  pthread_rwlock_wrlock(&trie->lock);
-
   // Generate random key
   bool ok = false;
   int key_length = secure ? RANDOM_KEY_LENGTH_LONG : RANDOM_KEY_LENGTH_SHORT;
@@ -389,7 +312,7 @@ char *trie_add_random(Trie *trie, Entry *entry, bool secure) {
     ok = trie_search_node(trie, key).child == NULL;
   }
 
-  bool res = trie_add_persistent(trie, key, entry);
+  bool res = trie_add(trie, key, entry);
   char *return_value;
 
   if (res) {
@@ -399,8 +322,6 @@ char *trie_add_random(Trie *trie, Entry *entry, bool secure) {
     free(key);
   }
 
-  pthread_rwlock_unlock(&trie->lock);
-
   return return_value;
 }
 
@@ -472,3 +393,12 @@ char *trie_add_random(Trie *trie, Entry *entry, bool secure) {
  * @return size of the trie
  */
 size_t trie_size(Trie *trie) { return trie->size; }
+
+int trie_rlock(Trie *trie) { return pthread_rwlock_rdlock(&trie->lock); }
+
+int trie_wlock(Trie *trie) { return pthread_rwlock_wrlock(&trie->lock); }
+
+int trie_unlock(Trie *trie) {
+  printf("sup\n");
+  return pthread_rwlock_unlock(&trie->lock);
+}

trie/src/trie_entry.c

@@ -0,0 +1,37 @@
+#include "trie.h"
+#include <stdlib.h>
+
+EntryType entry_type_from_char(char c) {
+  switch (c) {
+  case '0':
+    return Redirect;
+  case '1':
+    return Paste;
+  default:
+    return Unknown;
+  }
+}
+
+char entry_type_to_char(EntryType et) {
+  switch (et) {
+  case Redirect:
+    return '0';
+  case Paste:
+    return '1';
+  default:
+    return '\0';
+  }
+}
+
+Entry *entry_new(EntryType type, const char *string) {
+  Entry *entry = malloc(sizeof(Entry));
+  entry->type = type;
+
+  if (string != NULL) {
+    entry->string = strdup(string);
+  } else {
+    entry->string = NULL;
+  }
+
+  return entry;
+}