[Bug]: memory leaks can't delete or even destructor

EbadiDev commented 1 year ago

bit7z version

3.1.x

7-zip version

v19.00

7-zip DLL used

7z.dll

MSVC version

2022

Architecture

x86_64, x86

Which version of Windows are you using?

Windows 10

Bug description

so basically I allocates memory by using new but i couldn't delete it or destructor it it's gave me error of

Exception thrown: read access violation.
this->mInArchive-> was 0x5B8A8D00.

and show me this code

BitInputArchive::~BitInputArchive() {
    if ( mInArchive ) {
        mInArchive->Release();
    }
}

error comes from mInArchive->Release(); i am using bit7z for reading and extracting archive files and this error only come when i am reading archives metadata(extracting is fine even it's same code just other functions to use)

here full code of reading archive metadata:

#pragma once

#include "bitexception.hpp"
#include "bitarchiveinfo.hpp"
#include <iostream>
#include <string>
#include <windows.h>
#include <stdexcept> 
#include "getArchType.h"
#include "hash.h"
#include <shellapi.h>
//#include "crc.h"
//#include <filesystem>
//#include <iomanip>

using namespace  bit7z;

constexpr auto USER_INPUT = 255;

int archiveInfo(std::wstring dir) {
    try {
        /* 
        *
        * check system architecture
        * if x86 == load 7z.dll(x86)
        * if x64 == load 7z.dll(x64)
        * On x64 platforms sizeof(void*) returns 8. 
        * On x32 platforms sizeof(void*) returns 4.
        * 
        * */
        if (dir.size() > USER_INPUT)
            throw std::invalid_argument("argument too large.");

        BOOL windowsIs32Bit;
        BOOL isWOW64;
        BOOL processIs32Bit;

        if (!getBits(windowsIs32Bit, isWOW64, processIs32Bit)) {
            return -1;
        }
        Bit7zLibrary* lib = nullptr;

        //// Bit7zLibrary lib2;
        //// lib2 = new Bit7zLibrary(L"C:\\path-to-lib\\7z.dll");

        if (!windowsIs32Bit) {
            lib = new Bit7zLibrary(L"C:\\path-to-lib\\7z.dll");
        }
        else if (windowsIs32Bit) {
            lib = new Bit7zLibrary(L"C:\\path-to-lib\\7z.dll");
        }
        else {
            throw std::invalid_argument("arch type is not supported.");
        }
        //lib->setLargePageMode();

        BitArchiveInfo arc{ *lib, dir, BitFormat::Auto };

        // printing archive metadata
        // std::wcout << L"Archive properties" << std::endl;
        // Archive properties
        std::wstring tmp;
        std::wstring hash_args;
        std::wcout << arc.itemsCount() << std::endl; //  Items count: 
        std::wcout << arc.foldersCount() << std::endl;// Folders count:
        std::wcout << arc.filesCount() << std::endl;//   Files count: 
        std::wcout << arc.size() << std::endl;//         Size:
        std::wcout << arc.packSize() << std::endl;//     Packed size:

        std::wcout << std::endl;

        // Archive items

        auto arc_items = arc.items();
        long int filesize = 0;
        for (auto& item : arc_items) {
            //std::wcout << std::endl;
            std::wcout << item.index() << std::endl; //     Item index: 
            std::wcout << item.name() << std::endl; //      Name:
            std::wcout << item.extension() << std::endl; // Extension: 
            std::wcout << item.path() << std::endl; //      Path: 
            std::wcout << item.isDir() << std::endl; //     IsDir:
            std::wcout << item.size() << std::endl; //      Size: 
            std::wcout << item.packSize() << std::endl; //  Packed size:

            if (item.extension() != L""){
                // MD5 C++ WINAPI
                tmp = item.path();
                std::wcout << tmp << std::endl;
                std::cout << getHash(tmp) << std::endl;

                // MD5 CMD
                //hash_args = L"certutil -hashfile " + item.path() + L" " + L"md5";
                //_wsystem(hash_args.c_str());
                // 
                // CRC 32
                //data = (char*)malloc(item.size());
                //crc = CRC::Calculate(data, item.size(), CRC::CRC_32());
                //std::cout << std::hex << std::setw(8) << std::setfill('0') << crc << std::endl;

            }
            std::cout << "\n\n\n\n";
        }
        delete lib; // delete allocate we created with *new* 

        //lib ->~Bit7zLibrary();
    }

    catch (const BitException& ex) { //        for dll not loaded
        std::cerr << ex.what() << std::endl;
        return -1;
    }
    catch (const std::invalid_argument& e){ // for limit the string length
        std::cerr << e.what() << std::endl;
        return -1;
    }
    catch (...) {
        std::cerr << "Found an unknown exception." << std::endl;
        return -1;
    }

    return 0;
}

delete lib; or lib ->~Bit7zLibrary(); are giving me Exception thrown: read access violation. how to fix that?

is there anyway to load dll one time after checking system architecture ?

Steps to reproduce

No response

Expected behavior

No response

Relevant compilation output

No response

Code of Conduct

[X] By submitting this issue, I agree to follow bit7z's Code of Conduct

rikyoz commented 1 year ago

Hi!

delete lib; or lib ->~Bit7zLibrary(); are giving me Exception thrown: read access violation. how to fix that?

I think that the problem in your code is that the BitArchiveInfo object references the Bit7zLibrary, but then you delete the latter before the first goes out of scope. In other words, your code calls the library destructor before the BitArchiveInfo destructor is called, which tries to use the library and ultimately leads to the issue you're having. To fix this, you should enclose the part of the code that is using the BitArchiveInfo object in a more inner scope:

...
        { // BitArchiveInfo scope
            BitArchiveInfo arc{ *lib, dir, BitFormat::Auto };
            // code using the arc instance
        } // BitArchiveInfo's destructor is called
        delete lib; // Calling Bit7zLibrary's destructor
...

This will ensure that the destructors will be called in the correct order. In general, Bit7zLibrary instances must be the last to be destroyed!

is there anyway to load dll one time after checking system architecture ?

I can think of two possible ways:

If you really need to check the architecture at runtime, you could do something like this:

    ...
    std::wstring lib_path;
    if (!windowsIs32Bit) {
        lib_path = L"C:\\path-to-64-lib\\7z.dll";
    }
    else if (windowsIs32Bit) {
        lib_path = L"C:\\path-to-32-lib\\7z.dll";
    }
    else {
        throw std::invalid_argument("arch type is not supported.");
    }

    Bit7zLibrary lib{ lib_path };
    BitArchiveInfo arc{ lib, dir, BitFormat::Auto };
    ...

Or you could check the (executable) architecture at compile time:

    ...
    // Note: you might have to include the <cstdint> header to have these preprocessor defines available
    #if SIZE_MAX == UINT64_MAX
    Bit7zLibrary lib{ L"C:\\path-to-64-lib\\7z.dll" }; // 64 bit
    #else
    Bit7zLibrary lib{ L"C:\\path-to-32-lib\\7z.dll" }; // 32 bit
    #endif

    BitArchiveInfo arc{ lib, dir, BitFormat::Auto };
    ...

Please note that, in both these cases, there's no need to allocate the lib object on the heap (through new). This also makes it possible to avoid the previous scope fix since you do not have to call the library destructor explicitly; the program will already call the destructors in the correct order.

EbadiDev commented 1 year ago

Thank you so much, that's help me a lot and by the way, is there any way to get CRC32 hash from Bit7zLib ? I saw CRC in bitpropvariant.hpp, but I'm not familiar with enum classes or how to use them is there any example how I can get CRC32 hash when I am reading metadata from archive?

rikyoz commented 1 year ago

Thank you so much, that's help me a lot

You're welcome! Please note that in the last code example, I made a small mistake: it was #if, not #ifdef. I fixed it.

and by the way, is there any way to get CRC32 hash from Bit7zLib ? I saw CRC in bitpropvariant.hpp, but I'm not familiar with enum classes or how to use them is there any example how I can get CRC32 hash when I am reading metadata from archive?

You can get the CRC32 hash of an archive's item as follows:

uint32_t crc32 = 0;
auto crc_property = item.getProperty( BitProperty::CRC ); // BitPropVariant object
if ( crc_property.isUInt32() ) { // Just to be sure that the variant value has the correct type.
    crc32 = crc_property.getUInt32();
}

As far as I know, CRC32 variant values returned by 7-zip are either of UInt32 or Empty type, so you might need to check only if !crc_property.isEmpty().

EbadiDev commented 1 year ago

Thanks, Every thing works well now

rikyoz commented 1 year ago

You're welcome! :)

rikyoz / bit7z