Fix memory issues (emscripten-forge#33)

* Refreshin memory view

* Fix merge conflicts

* Minor fix

* fix memory issues

* fix memory issues

* fix memory issues

* minor fix of cleaning

* Fix cleaning memory

* Fix conflicts

* Cleaning code, replace error_message pointer

---------

Co-authored-by: martinRenou <[email protected]>

Author: AnastasiaSliusar

build_wasm.sh

@@ -84,9 +84,10 @@ emcc unpack.c -o $WASM_LIB/unpack.js \
     $CPPFLAGS $LDFLAGS \
     ${PREFIX}/lib/libarchive.a \
     ${PREFIX}/lib/libz.a ${PREFIX}/lib/libbz2.a ${PREFIX}/lib/libzstd.a ${PREFIX}/lib/libiconv.a\
-    -s MODULARIZE=1 -s WASM=1 -O3 -s ALLOW_MEMORY_GROWTH=1 \
+    -s MODULARIZE=1 -s WASM=1 -O3 -s ALLOW_MEMORY_GROWTH=1 -fsanitize=address \
+    -s INITIAL_MEMORY=128MB \
     -s ENVIRONMENT=web \
-    -s EXPORTED_RUNTIME_METHODS='["ccall", "cwrap", "getValue", "UTF8ToString"]' \
-    -s EXPORTED_FUNCTIONS="['_extract_archive', '_malloc', '_free']"
+    -s EXPORTED_RUNTIME_METHODS='["ccall", "cwrap", "getValue", "UTF8ToString", "wasmMemory"]' \
+    -s EXPORTED_FUNCTIONS="['_extract_archive', '_free_extracted_archive', '_malloc', '_free']"
 
 echo "Build completed successfully!"

src/index.ts

@@ -13,18 +13,18 @@ const fetchByteArray = async (url: string): Promise<Uint8Array> => {
 export const initUntarJS = async (): Promise<IUnpackJSAPI> => {
   const wasmModule = await initializeWasm();
 
-  const extractData = (data: Uint8Array): FilesData => {
+  const extractData = async (data: Uint8Array): Promise<FilesData> => {
     /**Since WebAssembly, memory is accessed using pointers
       and the first parameter of extract_archive method from unpack.c, which is Uint8Array of file data, should be a pointer
       so we have to allocate memory for file data
     **/
-    const inputPtr = wasmModule._malloc(data.length);
+    let inputPtr: number | null = wasmModule._malloc(data.length);
     wasmModule.HEAPU8.set(data, inputPtr);
 
     // fileCountPtr is the pointer to 4 bytes of memory in WebAssembly's heap that holds fileCount value from the ExtractedArchive structure in unpack.c.
-    const fileCountPtr = wasmModule._malloc(4);
+    let fileCountPtr: number | null = wasmModule._malloc(4);
 
-    const resultPtr = wasmModule._extract_archive(
+    let resultPtr: number | null = wasmModule._extract_archive(
       inputPtr,
       data.length,
       fileCountPtr
@@ -49,8 +49,8 @@ export const initUntarJS = async (): Promise<IUnpackJSAPI> => {
       and in order to get pointer of statusPtr we need to calculate it as: 0(offset of file pointer) + 4 (offset of fileCount) + 4 (offset for status)
       'status' field and pointer of `error_message` are 32-bit signed integer
     */
-    const statusPtr = wasmModule.getValue(resultPtr + 8, 'i32');
-    const errorMessagePtr = wasmModule.getValue(resultPtr + 12, 'i32');
+    let statusPtr: number | null = wasmModule.getValue(resultPtr + 8, 'i32');
+    let errorMessagePtr: number | null = resultPtr + 12;
     if (statusPtr !== 1) {
       const errorMessage = wasmModule.UTF8ToString(errorMessagePtr);
       console.error(
@@ -59,7 +59,14 @@ export const initUntarJS = async (): Promise<IUnpackJSAPI> => {
         'Error:',
         errorMessage
       );
-      return {};
+      wasmModule._free(inputPtr);
+      wasmModule._free(fileCountPtr);
+      wasmModule._free_extracted_archive(resultPtr);
+      inputPtr = null;
+      fileCountPtr = null;
+      resultPtr = null;
+      errorMessagePtr = null;
+      throw new Error(errorMessage);
     }
     const filesPtr = wasmModule.getValue(resultPtr, 'i32');
     const fileCount = wasmModule.getValue(resultPtr + 4, 'i32');
@@ -95,15 +102,19 @@ export const initUntarJS = async (): Promise<IUnpackJSAPI> => {
         dataPtr,
         dataSize
       );
+      
+      const fileDataCopy = fileData.slice(0);
 
-      files[filename] = fileData;
+      files[filename] = fileDataCopy;
     }
-
+  
     wasmModule._free(inputPtr);
     wasmModule._free(fileCountPtr);
-    wasmModule._free(errorMessagePtr);
-    wasmModule._free(resultPtr);
-
+    wasmModule._free_extracted_archive(resultPtr);
+    inputPtr = null;
+    fileCountPtr = null;
+    resultPtr = null;
+    errorMessagePtr = null;
     return files;
   };
 

src/types.ts

@@ -1,6 +1,6 @@
-export type FilesData = {[filename: string]: Uint8Array};
+export type FilesData = { [filename: string]: Uint8Array };
 
 export interface IUnpackJSAPI {
-  extractData: (data: Uint8Array) => FilesData;
+  extractData: (data: Uint8Array) => Promise<FilesData>;
   extract: (url: string) => Promise<FilesData>;
 }

src/unpack.d.ts

@@ -1,4 +1,6 @@
 export interface IWasmModule {
+  wasmMemory: any;
+  _free_extracted_archive(resultPtr: number): void;
   UTF8ToString(filenamePtr: number): string;
   HEAPU8: Uint8Array;
   _malloc(size: number): number;

unpack.c

@@ -15,7 +15,7 @@ typedef struct {
     FileData* files;
     size_t fileCount;
     int status;
-    char* error_message;
+    char error_message[256];
 } ExtractedArchive;
 
 EMSCRIPTEN_KEEPALIVE
@@ -27,39 +27,37 @@ ExtractedArchive* extract_archive(uint8_t* inputData, size_t inputSize, size_t*
 
     ExtractedArchive* result = (ExtractedArchive*)malloc(sizeof(ExtractedArchive));
     if (!result) {
-        result->status = 0;
-        result->error_message = strdup("Memory allocation error for ExtractedArchive.");
         return NULL;
     }
 
     result->files = NULL;
     result->fileCount = 0;
     result->status = 1;
-    result->error_message = NULL;
+    result->error_message[0] = '\0';
 
     archive = archive_read_new();
     archive_read_support_filter_all(archive);
     archive_read_support_format_all(archive);
 
     if (archive_read_open_memory(archive, inputData, inputSize) != ARCHIVE_OK) {
-        archive_read_free(archive);
         result->status = 0;
-        result->error_message = strdup(archive_error_string(archive));
+        snprintf(result->error_message, sizeof(result->error_message), "%s", archive_error_string(archive));
+        archive_read_free(archive);
         return result;
     }
 
     while (archive_read_next_header(archive, &entry) == ARCHIVE_OK) {
         const char* filename = archive_entry_pathname(entry);
         size_t entrySize = archive_entry_size(entry);
 
-        files = realloc(files, sizeof(FileData) * (files_count + 1));
+   
+        files= realloc(files, sizeof(FileData) * (files_count + 1));
         if (!files) {
             archive_read_free(archive);
             result->status = 0;
-            result->error_message = strdup("Memory allocation error for FileData array.");
+            snprintf(result->error_message, sizeof(result->error_message), "Memory allocation error for file data.");
             return result;
         }
-
         files[files_count].filename = strdup(filename);
         files[files_count].data = malloc(entrySize);
         files[files_count].data_size = entrySize;
@@ -68,7 +66,7 @@ ExtractedArchive* extract_archive(uint8_t* inputData, size_t inputSize, size_t*
             free(files[files_count].filename);
             archive_read_free(archive);
             result->status = 0;
-            result->error_message = strdup("Memory allocation error for file data.");
+            snprintf(result->error_message, sizeof(result->error_message), "Memory allocation error for file data.");
             return result;
         }
 
@@ -81,9 +79,9 @@ ExtractedArchive* extract_archive(uint8_t* inputData, size_t inputSize, size_t*
                     free(files[i].data);
                 }
                 free(files);
-                archive_read_free(archive);
                 result->status = 0;
-                result->error_message = strdup(archive_error_string(archive));
+                snprintf(result->error_message, sizeof(result->error_message), "%s", archive_error_string(archive));
+                archive_read_free(archive);
                 return result;
             }
             bytesRead += ret;
@@ -99,4 +97,15 @@ ExtractedArchive* extract_archive(uint8_t* inputData, size_t inputSize, size_t*
 }
 
 
-
+EMSCRIPTEN_KEEPALIVE
+void free_extracted_archive(ExtractedArchive* archive) {
+    if (!archive) {
+            fprintf(stderr, "No archive\n");
+    }
+    for (size_t i = 0; i < archive->fileCount; i++) {
+        free(archive->files[i].filename);
+        free(archive->files[i].data);
+    }
+    free(archive->files);
+    free(archive);
+}