Skip to content

Commit fac27ce

Browse files
committed
Fix concurrency issue in dayCount
The dayCount function was memoized, but there was no lock on the memoization dictionary, so several of the date functions would fail when called concurrently. Added tests to show this, and fixed the actual issue.
1 parent 060cc2d commit fac27ce

File tree

3 files changed

+75
-7
lines changed

3 files changed

+75
-7
lines changed

src/ExcelFinancialFunctions/common.fs

+9-7
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,15 @@ module internal Common =
119119
else
120120
let lower, upper = findBounds f guess -1.0 Double.MaxValue precision
121121
bisection f lower upper 0 precision
122-
122+
123123
let memoize f =
124124
let m = new Dictionary<_,_> ()
125125
fun x ->
126-
let foundIt, res = m.TryGetValue(x)
127-
if foundIt then res
128-
else
129-
let r = f x
130-
m.Add(x, r)
131-
r
126+
lock m (fun () ->
127+
let foundIt, res = m.TryGetValue(x)
128+
if foundIt then res
129+
else
130+
let r = f x
131+
m.Add(x, r)
132+
r
133+
)

tests/ExcelFinancialFunctions.Tests/ExcelFinancialFunctions.Tests.fsproj

+1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
<Compile Include="testutils.fs" />
4444
<Compile Include="crosstests.fs" />
4545
<Compile Include="spottests.fs" />
46+
<Compile Include="concurrencytests.fs" />
4647
</ItemGroup>
4748
<ItemGroup>
4849
<Reference Include="ExcelFinancialFunctions">
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
namespace Excel.FinancialFunctions.Tests
2+
3+
open NUnit.Framework
4+
5+
// These tests check that nothing goes wrong with the memoization which is used in the internal dayCount function
6+
// that is called by the functions tested below - when used concurrently.
7+
8+
[<SetCulture("en-US")>]
9+
module ConcurrencyTests =
10+
open System
11+
open Excel.FinancialFunctions
12+
open TestPreconditions
13+
14+
let parallelCount = 8
15+
16+
let TestParallel (f : unit -> unit) =
17+
let expected = true
18+
let actual =
19+
try
20+
[|1..parallelCount|]
21+
|> Array.Parallel.iter (fun _ -> f())
22+
true
23+
with
24+
| _ -> false
25+
26+
Assert.AreEqual(expected, actual)
27+
28+
let startDate = DateTime(2000, 1, 1)
29+
let endDate = DateTime(2010, 1, 1)
30+
31+
[<Test>]
32+
let YearFracWorksConcurrently() =
33+
let f = fun () -> (Financial.YearFrac(startDate, endDate, DayCountBasis.Actual365) |> ignore)
34+
TestParallel f
35+
36+
[<Test>]
37+
let CoupDaysWorksConcurrently() =
38+
let f = fun () -> (Financial.CoupDays(startDate, endDate, Frequency.Quarterly, DayCountBasis.Actual365) |> ignore)
39+
TestParallel f
40+
41+
[<Test>]
42+
let CoupPCDWorksConcurrently() =
43+
let f = fun () -> (Financial.CoupPCD(startDate, endDate, Frequency.Quarterly, DayCountBasis.Actual365) |> ignore)
44+
TestParallel f
45+
46+
[<Test>]
47+
let CoupNCDWorksConcurrently() =
48+
let f = fun () -> (Financial.CoupNCD(startDate, endDate, Frequency.Quarterly, DayCountBasis.Actual365) |> ignore)
49+
TestParallel f
50+
51+
[<Test>]
52+
let CoupNumWorksConcurrently() =
53+
let f = fun () -> (Financial.CoupNum(startDate, endDate, Frequency.Quarterly, DayCountBasis.Actual365) |> ignore)
54+
TestParallel f
55+
56+
[<Test>]
57+
let CoupDaysBSWorksConcurrently() =
58+
let f = fun () -> (Financial.CoupDaysBS(startDate, endDate, Frequency.Quarterly, DayCountBasis.Actual365) |> ignore)
59+
TestParallel f
60+
61+
[<Test>]
62+
let CoupDaysNCWorksConcurrently() =
63+
let f = fun () -> (Financial.CoupDaysNC(startDate, endDate, Frequency.Quarterly, DayCountBasis.Actual365) |> ignore)
64+
TestParallel f
65+

0 commit comments

Comments
 (0)