Skip to content

Commit 1d13913

Browse files
committed
fix gc-caused access violation on delegates for comparers, fixes #41
1 parent cd0cffa commit 1d13913

File tree

2 files changed

+56
-13
lines changed

2 files changed

+56
-13
lines changed

RocksDbSharp/ColumnFamilyOptions.cs

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
using System.Runtime.InteropServices;
66
using System.Text;
77
using System.Threading.Tasks;
8+
using Transitional;
89

910
namespace RocksDbSharp
1011
{
1112
public class ColumnFamilyOptions : OptionsHandle
1213
{
13-
Comparator Comparator { get; set; }
14-
IntPtr ComparatorStatePtr { get; set; }
14+
ComparatorReferences ComparatorRef { get; set; }
1515

1616
public ColumnFamilyOptions SetBlockBasedTableFactory(BlockBasedTableOptions table_options)
1717
{
@@ -169,51 +169,72 @@ public ColumnFamilyOptions SetComparator(IntPtr comparator)
169169
/// </summary>
170170
public ColumnFamilyOptions SetComparator(Comparator comparator)
171171
{
172-
// Hold onto the reference to the comparator
173-
Comparator = comparator;
174-
175172
// Allocate some memory for the name bytes
176173
var name = comparator.Name ?? comparator.GetType().FullName;
177174
var nameBytes = Encoding.UTF8.GetBytes(name + "\0");
178175
var namePtr = Marshal.AllocHGlobal(nameBytes.Length);
179176
Marshal.Copy(nameBytes, 0, namePtr, nameBytes.Length);
180177

178+
// Hold onto a reference to everything that needs to stay alive
179+
ComparatorRef = new ComparatorReferences
180+
{
181+
GetComparator = () => comparator,
182+
CompareDelegate = Comparator_Compare,
183+
DestructorDelegate = Comparator_Destroy,
184+
NameDelegate = Comparator_GetNamePtr,
185+
};
186+
181187
// Allocate the state
182188
var state = new ComparatorState
183189
{
184190
NamePtr = namePtr,
191+
GetComparatorPtr = CurrentFramework.GetFunctionPointerForDelegate<GetComparator>(ComparatorRef.GetComparator)
185192
};
186193
var statePtr = Marshal.AllocHGlobal(Marshal.SizeOf(state));
187194
Marshal.StructureToPtr(state, statePtr, false);
188195

189196
// Create the comparator
190197
IntPtr handle = Native.Instance.rocksdb_comparator_create(
191198
state: statePtr,
192-
destructor: Comparator_Destroy,
193-
compare: Comparator_Compare,
194-
name: Comparator_GetNamePtr
199+
destructor: ComparatorRef.DestructorDelegate,
200+
compare: ComparatorRef.CompareDelegate,
201+
name: ComparatorRef.NameDelegate
195202
);
196203

197-
ComparatorStatePtr = statePtr;
198-
199204
return SetComparator(handle);
200205
}
201206

207+
delegate Comparator GetComparator();
208+
private class ComparatorReferences
209+
{
210+
public GetComparator GetComparator { get; set; }
211+
public DestructorDelegate DestructorDelegate { get; set; }
212+
public CompareDelegate CompareDelegate { get; set; }
213+
public NameDelegate NameDelegate { get; set; }
214+
}
215+
202216
private unsafe int Comparator_Compare(IntPtr state, IntPtr a, UIntPtr alen, IntPtr b, UIntPtr blen)
203-
=> Comparator.Compare(a, alen, b, blen);
217+
{
218+
var getComparatorPtr = (*((ComparatorState*)state)).GetComparatorPtr;
219+
var getComparator = CurrentFramework.GetDelegateForFunctionPointer<GetComparator>(getComparatorPtr);
220+
var comparator = getComparator();
221+
return comparator.Compare(a, alen, b, blen);
222+
}
204223

205-
private unsafe void Comparator_Destroy(IntPtr state)
224+
private unsafe static void Comparator_Destroy(IntPtr state)
206225
{
207226
var namePtr = (*((ComparatorState*)state)).NamePtr;
208227
Marshal.FreeHGlobal(namePtr);
209228
Marshal.FreeHGlobal(state);
210229
}
211-
private unsafe IntPtr Comparator_GetNamePtr(IntPtr state)
230+
231+
private unsafe static IntPtr Comparator_GetNamePtr(IntPtr state)
212232
=> (*((ComparatorState*)state)).NamePtr;
213233

214234
[StructLayout(LayoutKind.Sequential)]
215235
private struct ComparatorState
216236
{
237+
public IntPtr GetComparatorPtr { get; set; }
217238
public IntPtr NamePtr { get; set; }
218239
}
219240

tests/RocksDbSharpTest/FunctionalTests.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,28 @@ public void FunctionalTest()
357357
}
358358
}
359359

360+
// Test that GC does not cause access violation on Comparers
361+
{
362+
if (Directory.Exists("test-av-error"))
363+
Directory.Delete("test-av-error", true);
364+
options = new RocksDbSharp.DbOptions()
365+
.SetCreateIfMissing(true)
366+
.SetCreateMissingColumnFamilies(true);
367+
var sc = new RocksDbSharp.StringComparator(StringComparer.InvariantCultureIgnoreCase);
368+
columnFamilies = new RocksDbSharp.ColumnFamilies
369+
{
370+
{ "cf1", new RocksDbSharp.ColumnFamilyOptions()
371+
.SetComparator(sc)
372+
},
373+
};
374+
GC.Collect();
375+
using (var db = RocksDbSharp.RocksDb.Open(options, "test-av-error", columnFamilies))
376+
{
377+
}
378+
if (Directory.Exists("test-av-error"))
379+
Directory.Delete("test-av-error", true);
380+
}
381+
360382
}
361383

362384
class IntegerStringComparator : StringComparatorBase

0 commit comments

Comments
 (0)