cpp:S2095 reported erroneously when using 'wil' smart resource pointers

Rules and Languages Report False-positive / False-negative...
,
1

The ‘wil’ library provides smart resource pointers/wrappers somewhat similar to std::unique_ptr but for Windows resource handles - i.e. the smart pointer takes care of closing the handle when the variable goes out of scope. (See GitHub - microsoft/wil: Windows Implementation Library). But SonarQube is reporting a possible resource leak when using these objects. (cpp:S2095 Resources should be closed).

This can be demonstrated with a very simple example.

#include <iostream>
#include <wil/resource.h>

void s2095(const char* fileName)
{
    auto hFile = wil::unique_hfile{CreateFile(fileName, FILE_READ_ATTRIBUTES, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, 0, nullptr)};
    if (!hFile)
    {
        std::cerr << "Unable to open file " << fileName << ". Error: " << GetLastError() << std::endl;
    }
    else
    {
        auto createTime = FILETIME{};
        auto lastWriteTime = FILETIME{};
        if (GetFileTime(hFile.get(), &createTime, nullptr, &lastWriteTime))
        {
            std::cout << "lastWriteTime: " << lastWriteTime.dwLowDateTime << "," << lastWriteTime.dwHighDateTime << std::endl;
        }
    }
} // cpp:S2095 reported erroneously here

Observed with SonarQube for Visual Studio 2022 and SonarQube Server Enterprise v2025.1

4

Hello, @giles45

Thank you for reporting a false positive!
As I have never used wil library, it is difficult for me to reproduce the issue. Could you, please, create a reproducer file that captures all the compilation options and relevant source code?
To generate the reproducer file:

  • Search in the analysis log for the full path of the source file for which you want to create a reproducer (for instance, a file that contains a false-positive). You will have to use exactly this name (same case, / or \…)
  • Add the reproducer option to the scanner configuration:
    sonar.cfamily.reproducer=“Full path to the .source file”
  • Re-run the scanner to generate a file named sonar-cfamily-reproducer.zip in the project folder.
  • Please share this file. If you think this file contains private information, let us know, and we’ll send you a private message that will allow you to send it privately.
6

Thanks for the response.

‘wil’ is a header only library so not difficult to install or use - but as requested I’ve generated a reproducer file. I generated this differently from how you said so I could provide the reproducer file for the example shown. I used this command Analyze.SonarLint.CFamily.CreateReproducer from within Visual Studio (from Google). If this is not right, let me know. But this is the file attached.

sonar-cfamily-reproducer.tarxz (1.1 MB)

1 Like
7

Thank you for the reproducer file. With its help, I was able to reproduce the bug report.
It appears to me that wil has a bug here, reported by Sonar as expected. (I do agree that it could provide a better explanation, though)

Here is how I understand the code:

  1. Upon destruction of hFile compiler invokes the destructor of wil::unique_hfile (wil::unique_hfile::~unique_hfile()), which is is defined here to unique_any_handle_invalid<decltype(&::CloseHandle), ::CloseHandle>::~unique_any_handle_invalid<decltype(&::CloseHandle), ::CloseHandle>().
  2. unique_any_handle_invalid is defined here as
    template <typename close_fn_t, close_fn_t close_fn>
using unique_any_handle_invalid = 
  unique_any_t<
    details::unique_storage<
        details::handle_invalid_resource_policy<close_fn_t, close_fn>>>;
  3. Destructor of unique_any_t invokes the destructor of its template parameter storage_t following the inhritance as you can see in its definition. In this case storage_t is details::unique_storage<details::handle_invalid_resource_policy<close_fn_t, close_fn>.
  4. Destructor details::unique_storage::~unique_storage() invokes policy::close only if policy::is_valid is true:
        ~unique_storage() WI_NOEXCEPT
    {
        if (policy::is_valid(m_ptr))
        {
            policy::close(m_ptr);
        }
    }
  5. policy = details::handle_invalid_resource_policy is defined here, note its is_valid static member function has two conditions:
        __forceinline static bool is_valid(HANDLE ptr) WI_NOEXCEPT
    {
        return ((ptr != INVALID_HANDLE_VALUE) && (ptr != nullptr));
    }

CreateFile returns either a file handle for an open file or INVALID_HANDLE_VALUE (which is -1, BTW), see MSDN. So, nullptr is a possible return file for a successfuly opened file.

Bottom line is: If CreateFile returns nullptr, will::unique_hfile` will not automatically close the file, so it will leak the file handle, which is what our rule warns you about.

8

Yes, I agree with your analysis of what the destructor does. The question is whether nullptr could ever be returned as a valid handle. It doesn’t seem sensible that it would but I know the documentation doesn’t say otherwise. I’ve wondered this before. Given this library is written by MS and I believe it’s what they use internally, I’ve assumed they know best. I’ll post a query on the wil github page.

9

Link to the issue raised on the wil github page.

1 Like
10

Thanks! Let me know how it evolves. If Microsoft decides this is not a leak, we will amend the rule.