HeapFree is crashing

Question

Summary:

I have VBA code that collects lots of info and writes it out into one or more worksheets. To improve perf writing lots of info into sheets, I created a class that acts kind of like a buffered copy/paste stream: the caller sends it CSV format strings which it buffers in memory until the buffer is full; when full, it pastes into a sheet, clears the buffer and continues.

Initially, I used Global memory, but then saw on MSDN a recommendation to use Heap rather than Global or Local due to less overhead. So now I'm using Heap.

I'm in the process of adapting everything for 64-bit Office. After doing all the PtrSafe stuff, I can run the code. But now Excel crashes when HeapFree() is called.

Question: Why is it crashing, and what do I need to change to avoid it?

Details:

I've come up with the following that is a minimized sample that repro's the crash: there's a module with a sub I can run for the repro, and the class. This doesn't do the buffering; every call to .SendText will put the text on the clipboard and paste into the active cell.

First the module. This has the following declare statements

' memory APIs
Public Const HEAP_ZERO_MEMORY = &H8

Declare PtrSafe Function GetProcessHeap Lib "kernel32" () As LongPtr 'returns HANDLE
Declare PtrSafe Function HeapAlloc Lib "kernel32" (ByVal hHeap As LongPtr, ByVal dwFlags As Long, ByVal dwBytes As LongPtr) As LongPtr 'returns HANDLE
Declare PtrSafe Function HeapFree Lib "kernel32" (ByVal hHeap As LongPtr, ByVal dwFlags As Long, lpMem As Any) As Long 'returns BOOL

Declare PtrSafe Function lstrcpy Lib "kernel32" Alias "lstrcpyW" (ByVal lpDestString As Any, ByVal lpSrcString As Any) As LongPtr 'returns HANDLE

' clipboard APIs
Public Const CF_UNICODETEXT = 13

Declare PtrSafe Function OpenClipboard Lib "user32" (ByVal hwnd As LongPtr) As Long 'returns BOOL
Declare PtrSafe Function EmptyClipboard Lib "user32" () As Long 'returns BOOL
Declare PtrSafe Function CloseClipboard Lib "user32" () As Long 'returns BOOL
Declare PtrSafe Function SetClipboardData Lib "user32" (ByVal wFormat As Long, ByVal hMem As LongPtr) As LongPtr   'returns HANDLE

Then the sub:

Private Sub TestMemoryBugRepro()
    Dim s As String
    Dim clip As clsHeapBugRepro

    s = Chr(34) & "a" & vbLf & "b" & Chr(34) & ",c" & vbCrLf & "d" & vbTab & ",e" & vbCrLf

    Set clip = New clsHeapBugRepro
    clip.Initialize &H100
    clip.SendText s

    'Crash happens during the following
    Set clip = Nothing
End Sub

Now the class. The crash occurs during Class_Terminate() when HeapFree is called.

Option Explicit

Private m_hHeap As LongPtr 'handle to the process heap
Private m_hMem As LongPtr 'handle to memory
Private m_pMem As LongPtr 'pointer to locked memory
Private m_cbMem As Long 'size of the memory buffer
Private m_BytesWritten As Long '



'**************************************
'  Event procedures

Private Sub Class_Initialize()
    m_hHeap = GetProcessHeap()
End Sub

Private Sub Class_Terminate()
    If m_hMem <> 0 And m_hHeap <> 0 Then
        HeapFree m_hHeap, 0, m_hMem       'CRASH OCCURS HERE
    End If
End Sub


'**************************************
'  Public methods

Public Function Initialize(Optional bufferSize As Long = &H8000) As Boolean
    Initialize = False
    m_BytesWritten = 0

    If m_hHeap <> 0 Then
        m_cbMem = bufferSize
        m_hMem = HeapAlloc(m_hHeap, (HEAP_ZERO_MEMORY), m_cbMem)
    End If
    If m_hMem <> 0 Then Initialize = True
End Function


Public Function SendText(text As String) As Boolean
    Dim nStrLen As Long

    nStrLen = LenB(text) + 2&
    Debug.Assert nStrLen < (m_cbMem + m_BytesWritten)

    m_pMem = m_hMem 'in lieu of locking heap memory
    lstrcpy m_pMem, StrPtr(text)
    m_pMem = 0 'in lieu of unlocking heap memory
    m_BytesWritten = m_BytesWritten + nStrLen

    DoEvents

    OpenClipboard 0&
    EmptyClipboard
    SetClipboardData CF_UNICODETEXT, m_hMem
    CloseClipboard

    ActiveCell.PasteSpecial
    DoEvents

    SendText = True
End Function

You can't narrow it down any more than all of that code that no one is going to go thru for you? have you ever debugged a program before? do you not know how to set up breakpoints? — braX, May 31 '20 at 03:22

score 0 · Accepted Answer · edited Jun 07 '20 at 07:04

0

There's an issue in how you're calling HeapFree, but your real issue is that you shouldn't be using HeapAlloc/HeapFree at all.

Memory allocated by HeapAlloc is not movable, whereas SetClipboardData requires it to be moveable.

Another consideration is that SetClipboardData transfers the ownership of the memory to the system, which means you should not free it yourself. (The application may not write to or free the data once ownership has been transferred to the system.)

So, I would try to rewrite your logic using GlobalAlloc, not HeapAlloc, and don't try to free memory after it is put on the clipboard.

Now, there was an issue in how you were calling HeapFree, if you were going to use it. HeapFree wants the pointer returned by HeapAlloc.

You were instead passing a pointer to that pointer, because you declared the third argument of HeapFree As Any, which means ByRef As Any.

Either redeclare the argument ByVal As LongPtr / ByVal As Any, e.g.:

Declare PtrSafe Function HeapFree Lib "kernel32" (ByVal hHeap As LongPtr, _
                                                  ByVal dwFlags As Long,
                                                  ByVal lpMem As LongPtr) As Long 'returns BOOL

or specify ByVal when calling it:

HeapFree m_hHeap, 0, ByVal m_hMem

edited Jun 07 '20 at 07:04

Peter Constable

2,707
10
23

answered May 31 '20 at 22:32

GSerg

76,472
17
159
346

Adding ```ByVal``` alone on the argument in the ```HeapFree``` call did not stop the crash; and adding it only in the ```declare``` didn't stop the crash. But adding ```ByVal``` in both places does appear to fix it. – Peter Constable May 31 '20 at 23:12
@PeterConstable That cannot really be. Specifying `ByVal` in the function definition has higher precedence, and after that specifying or omitting `ByVal` in the call has no effect. Similarly, there is no way that with `ByVal` in the call it would be called `ByRef`, although I could potentially leave some doubts regarding the `Any`, in which case I would have also tested it with `ByVal As Any` instead of `ByVal As LongPtr`. Are you sure you were running the correct version of code when you observed that? Could it be that you have two `Declare`s for `HeapFree`? – GSerg Jun 01 '20 at 06:48
No, I created a minimalized .xlsm that only has one ```declare```.If I have ```Declare... ByVal lpMem As Any...``` and call ```HeapFree m_hHeap, 0, m_hMem```, it's fine if I single-step all the way through ```TestMemoryBugRepro```. But if I use ```F5``` to just run it, Excel crashes. – Peter Constable Jun 04 '20 at 01:35
I also tried changing to ```Declare... ByVal lpMem As LongPtr``` and calling ```HeapFree m_hHeap, 0, m_hMem```, and this time it crashed when running using ```F5``` or single-stepping. – Peter Constable Jun 04 '20 at 01:39
And I get the same results if I change the declare to ```Declare... lpMem as LongPtr``` but call ```HeapFree m_hHeap, 0, ByVal m_hMem```. So, it seems like I don't yet have a reliable fix. I don't understand what's wrong. – Peter Constable Jun 04 '20 at 01:45
@PeterConstable I'm also confused. I believe it may have something to do with the fact that [*Memory allocated by HeapAlloc is not movable*](https://learn.microsoft.com/en-us/windows/win32/api/heapapi/nf-heapapi-heapalloc#remarks), whereas `SetClipboardData` [requires](https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setclipboarddata#parameters) it to be moveable. I would try to rewrite your logic using GlobalAlloc / GlobalFree. – GSerg Jun 04 '20 at 07:15
@PeterConstable Another consideration is that `SetClipboardData` transfers the ownership of the memory to the system, which means you [should not free it](https://stackoverflow.com/a/16039894/11683) yourself ([*The application may not write to or free the data once ownership has been transferred to the system*](https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setclipboarddata#parameters)). – GSerg Jun 04 '20 at 07:20
I had originally implemented this using Global functions. Writing lots of data into sheets was still a perf bottleneck, and I saw in MSDN a recommendation to use Heap rather than Global. But that recommendation kind of contradicts the guidance for ```SetClipboardData``` to use ```GMEM_MOVEABLE```. But I guess I need to go back to Global. I probably had missed the notes about transferred ownership; clearly something I need to fix. I'll try changes when I have a chance. – Peter Constable Jun 04 '20 at 15:32
I've changed back to using Global, have run several trials and not had any hangs. So, the use of heap as well as freeing after ```SetClipBoardData``` were likely causes. If you'd like to update your answer I can accept. (I wish I could get better perf, though: still ~15 seconds to write strings into <4000 cells on a sheet.) – Peter Constable Jun 07 '20 at 02:55
Wow!! I never knew I could write thousands of strings to a 2D variant array and assign that to a sheet range **so much faster** (>15s to <0.05s) than putting the strings into a memory buffer using winapi functions and pasting from the clipboard! Glad I tried out using the memory functions, but wish I new that years ago. – Peter Constable Jun 07 '20 at 05:14

HeapFree is crashing

1 Answers1