Skip to content

Commit 2f749f5

Browse files
shimatshimat
authored
Harden managed/native boundaries and CI inputs (#2087)
* Harden managed and native boundaries * Address callback and cache review feedback
1 parent 4998db0 commit 2f749f5

28 files changed

Lines changed: 580 additions & 284 deletions

‎.editorconfig‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ csharp_preferred_modifier_order = public,internal:suggestion
6262
# CA1711: Identifiers should not have incorrect suffix
6363
dotnet_diagnostic.CA1711.severity = none
6464

65-
# CA5393: Do not use unsafe DllImportSearchPath value
66-
dotnet_diagnostic.CA5393.severity = none
67-
6865
# https://docs.microsoft.com/ja-jp/dotnet/fundamentals/code-analysis/quality-rules/ca1051
6966
# TODO: not working??
7067
dotnet_diagnostic.ca1051.exclude_structs = true

‎.github/actions/build-manylinux-variant/action.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,13 @@ runs:
6767
- name: Bootstrap vcpkg (${{ inputs.variant_label }})
6868
shell: bash
6969
run: |
70-
git clone --filter=blob:none https://github.com/microsoft/vcpkg.git /opt/vcpkg
70+
set -euo pipefail
71+
vcpkg_commit="$(sed -n 's/.*"builtin-baseline": *"\([^"]*\)".*/\1/p' "${GITHUB_WORKSPACE}/vcpkg.json")"
72+
test -n "${vcpkg_commit}"
73+
git init /opt/vcpkg
74+
git -C /opt/vcpkg remote add origin https://github.com/microsoft/vcpkg.git
75+
git -C /opt/vcpkg fetch --depth 1 origin "${vcpkg_commit}"
76+
git -C /opt/vcpkg checkout --detach FETCH_HEAD
7177
/opt/vcpkg/bootstrap-vcpkg.sh -disableMetrics
7278
7379
- name: Restore vcpkg package cache (${{ inputs.variant_label }})

‎.github/workflows/macos.yml‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,13 @@ jobs:
4141
# -- vcpkg (image libs) --------------------------------------------------
4242
- name: Bootstrap vcpkg
4343
run: |
44-
git clone --filter=blob:none https://github.com/microsoft/vcpkg.git /tmp/vcpkg
44+
set -euo pipefail
45+
vcpkg_commit="$(sed -n 's/.*"builtin-baseline": *"\([^"]*\)".*/\1/p' "${GITHUB_WORKSPACE}/vcpkg.json")"
46+
test -n "${vcpkg_commit}"
47+
git init /tmp/vcpkg
48+
git -C /tmp/vcpkg remote add origin https://github.com/microsoft/vcpkg.git
49+
git -C /tmp/vcpkg fetch --depth 1 origin "${vcpkg_commit}"
50+
git -C /tmp/vcpkg checkout --detach FETCH_HEAD
4551
/tmp/vcpkg/bootstrap-vcpkg.sh -disableMetrics
4652
4753
- name: Restore vcpkg package cache
@@ -195,7 +201,13 @@ jobs:
195201
# -- vcpkg (image libs) --------------------------------------------------
196202
- name: Bootstrap vcpkg
197203
run: |
198-
git clone --filter=blob:none https://github.com/microsoft/vcpkg.git /tmp/vcpkg
204+
set -euo pipefail
205+
vcpkg_commit="$(sed -n 's/.*"builtin-baseline": *"\([^"]*\)".*/\1/p' "${GITHUB_WORKSPACE}/vcpkg.json")"
206+
test -n "${vcpkg_commit}"
207+
git init /tmp/vcpkg
208+
git -C /tmp/vcpkg remote add origin https://github.com/microsoft/vcpkg.git
209+
git -C /tmp/vcpkg fetch --depth 1 origin "${vcpkg_commit}"
210+
git -C /tmp/vcpkg checkout --detach FETCH_HEAD
199211
/tmp/vcpkg/bootstrap-vcpkg.sh -disableMetrics
200212
201213
- name: Restore vcpkg package cache

‎.github/workflows/publish_nuget.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ on:
1212
type: string
1313
default: ""
1414

15+
permissions:
16+
actions: read
17+
contents: read
18+
1519
jobs:
1620
Prepare:
1721
runs-on: ubuntu-24.04

‎.github/workflows/wasm.yml‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ jobs:
3030
- uses: actions/checkout@v7
3131
with:
3232
fetch-depth: 1
33+
persist-credentials: false
34+
submodules: recursive
35+
36+
- name: Resolve OpenCV revisions
37+
id: opencv-revisions
38+
run: |
39+
echo "opencv=$(git rev-parse HEAD:opencv)" >> "${GITHUB_OUTPUT}"
40+
echo "opencv_contrib=$(git rev-parse HEAD:opencv_contrib)" >> "${GITHUB_OUTPUT}"
3341
3442
- name: Install dependencies
3543
run: |
@@ -69,7 +77,7 @@ jobs:
6977
${{ github.workspace }}/opencv_wasm/include
7078
${{ github.workspace }}/opencv_wasm/lib
7179
${{ github.workspace }}/opencv_wasm/libopencv.o
72-
key: opencv-${{ env.OPENCV_VERSION }}-wasm-${{ env.EM_VERSION }}-v${{ env.OPENCV_CACHE_VERSION }}
80+
key: opencv-${{ steps.opencv-revisions.outputs.opencv }}-${{ steps.opencv-revisions.outputs.opencv_contrib }}-wasm-${{ env.EM_VERSION }}-v${{ env.OPENCV_CACHE_VERSION }}
7381

7482
- name: Setup Emscripten cache
7583
id: cache-system-libraries
@@ -85,13 +93,11 @@ jobs:
8593
- name: Build OpenCV
8694
if: steps.opencv-cache.outputs.cache-hit != 'true'
8795
run: |
88-
wget https://github.com/opencv/opencv/archive/${OPENCV_VERSION}.zip -Oopencv-${OPENCV_VERSION}.zip && unzip opencv-${OPENCV_VERSION}.zip
89-
wget https://github.com/opencv/opencv_contrib/archive/${OPENCV_VERSION}.zip -Oopencv_contrib-${OPENCV_VERSION}.zip && unzip opencv_contrib-${OPENCV_VERSION}.zip
9096
emcmake cmake \
91-
-S opencv-${OPENCV_VERSION} \
92-
-B opencv-${OPENCV_VERSION}/build \
97+
-S opencv \
98+
-B opencv/build \
9399
-DCMAKE_BUILD_TYPE=Release \
94-
-DOPENCV_EXTRA_MODULES_PATH=${GITHUB_WORKSPACE}/opencv_contrib-${OPENCV_VERSION}/modules \
100+
-DOPENCV_EXTRA_MODULES_PATH=${GITHUB_WORKSPACE}/opencv_contrib/modules \
95101
-DBUILD_SHARED_LIBS=OFF \
96102
-DENABLE_CXX11=ON -DBUILD_TESTS=OFF \
97103
-DBUILD_PERF_TESTS=OFF \
@@ -139,8 +145,8 @@ jobs:
139145
-DWITH_PTHREADS_PF=OFF \
140146
-DCV_ENABLE_INTRINSICS=OFF \
141147
-DBUILD_opencv_dnn=ON
142-
cmake --build opencv-${OPENCV_VERSION}/build --parallel $(nproc)
143-
cmake --install opencv-${OPENCV_VERSION}/build
148+
cmake --build opencv/build --parallel $(nproc)
149+
cmake --install opencv/build
144150
em++ -r -o ${GITHUB_WORKSPACE}/opencv_wasm/libopencv.o \
145151
-Wl,--whole-archive ${GITHUB_WORKSPACE}/opencv_wasm/lib/*.a \
146152
${GITHUB_WORKSPACE}/opencv_wasm/lib/opencv5/3rdparty/*.a
@@ -154,7 +160,7 @@ jobs:
154160
${{ github.workspace }}/opencv_wasm/include
155161
${{ github.workspace }}/opencv_wasm/lib
156162
${{ github.workspace }}/opencv_wasm/libopencv.o
157-
key: opencv-${{ env.OPENCV_VERSION }}-wasm-${{ env.EM_VERSION }}-v${{ env.OPENCV_CACHE_VERSION }}
163+
key: opencv-${{ steps.opencv-revisions.outputs.opencv }}-${{ steps.opencv-revisions.outputs.opencv_contrib }}-wasm-${{ env.EM_VERSION }}-v${{ env.OPENCV_CACHE_VERSION }}
158164

159165
- name: Build OpenCvSharpExtern
160166
run: |

‎src/OpenCvSharp/Cv2/Cv2_highgui.cs‎

Lines changed: 58 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -142,21 +142,25 @@ public static void DestroyWindow(string winName)
142142
if (string.IsNullOrEmpty(winName))
143143
throw new ArgumentException("null or empty string.", nameof(winName));
144144

145-
NativeMethods.HandleException(
146-
NativeMethods.highgui_destroyWindow(winName));
147-
148-
ForgetWindowCallbacks(winName);
145+
lock (highguiCallbackSync)
146+
{
147+
NativeMethods.HandleException(
148+
NativeMethods.highgui_destroyWindow(winName));
149+
ForgetWindowCallbacks(winName);
150+
}
149151
}
150152

151153
/// <summary>
152154
/// Destroys all of the HighGUI windows.
153155
/// </summary>
154156
public static void DestroyAllWindows()
155157
{
156-
NativeMethods.HandleException(
157-
NativeMethods.highgui_destroyAllWindows());
158-
159-
ForgetAllCallbacks();
158+
lock (highguiCallbackSync)
159+
{
160+
NativeMethods.HandleException(
161+
NativeMethods.highgui_destroyAllWindows());
162+
ForgetAllCallbacks();
163+
}
160164
}
161165

162166
/// <summary>
@@ -354,19 +358,22 @@ public static void SetMouseCallback(string windowName, MouseCallback onMouse, In
354358

355359
var context = new MouseCallbackContext { Callback = onMouse, UserData = userData };
356360
var contextHandle = GCHandle.Alloc(context);
357-
try
358-
{
359-
NativeMethods.HandleException(
360-
NativeMethods.highgui_setMouseCallback(windowName, GetMouseCallbackTrampolinePointer(), GCHandle.ToIntPtr(contextHandle)));
361-
}
362-
catch
361+
lock (highguiCallbackSync)
363362
{
364-
contextHandle.Free();
365-
throw;
366-
}
363+
try
364+
{
365+
NativeMethods.HandleException(
366+
NativeMethods.highgui_setMouseCallback(windowName, GetMouseCallbackTrampolinePointer(), GCHandle.ToIntPtr(contextHandle)));
367+
}
368+
catch
369+
{
370+
contextHandle.Free();
371+
throw;
372+
}
367373

368-
// Root the context (and thus the delegate) for the lifetime of the window (see registry note above).
369-
RegisterMouseCallback(windowName, contextHandle);
374+
// Root the context (and thus the delegate) for the lifetime of the window (see registry note above).
375+
RegisterMouseCallback(windowName, contextHandle);
376+
}
370377
}
371378

372379
/// <summary>
@@ -493,21 +500,24 @@ public static int CreateTrackbar(string trackbarName, string winName,
493500
}
494501

495502
int ret;
496-
try
497-
{
498-
NativeMethods.HandleException(
499-
NativeMethods.highgui_createTrackbar(
500-
trackbarName, winName, ref value, count, onChangePtr,
501-
contextHandle.HasValue ? GCHandle.ToIntPtr(contextHandle.Value) : IntPtr.Zero, out ret));
502-
}
503-
catch
503+
lock (highguiCallbackSync)
504504
{
505-
contextHandle?.Free();
506-
throw;
507-
}
505+
try
506+
{
507+
NativeMethods.HandleException(
508+
NativeMethods.highgui_createTrackbar(
509+
trackbarName, winName, ref value, count, onChangePtr,
510+
contextHandle.HasValue ? GCHandle.ToIntPtr(contextHandle.Value) : IntPtr.Zero, out ret));
511+
}
512+
catch
513+
{
514+
contextHandle?.Free();
515+
throw;
516+
}
508517

509-
// Root the context (and thus the delegate) for the lifetime of the window (see registry note above).
510-
RegisterTrackbarCallback(winName, trackbarName, contextHandle);
518+
// Root the context (and thus the delegate) for the lifetime of the window (see registry note above).
519+
RegisterTrackbarCallback(winName, trackbarName, contextHandle);
520+
}
511521
return ret;
512522
}
513523

@@ -542,21 +552,24 @@ public static int CreateTrackbar(string trackbarName, string winName,
542552
}
543553

544554
int ret;
545-
try
546-
{
547-
NativeMethods.HandleException(
548-
NativeMethods.highgui_createTrackbar(
549-
trackbarName, winName, IntPtr.Zero, count, onChangePtr,
550-
contextHandle.HasValue ? GCHandle.ToIntPtr(contextHandle.Value) : IntPtr.Zero, out ret));
551-
}
552-
catch
555+
lock (highguiCallbackSync)
553556
{
554-
contextHandle?.Free();
555-
throw;
556-
}
557+
try
558+
{
559+
NativeMethods.HandleException(
560+
NativeMethods.highgui_createTrackbar(
561+
trackbarName, winName, IntPtr.Zero, count, onChangePtr,
562+
contextHandle.HasValue ? GCHandle.ToIntPtr(contextHandle.Value) : IntPtr.Zero, out ret));
563+
}
564+
catch
565+
{
566+
contextHandle?.Free();
567+
throw;
568+
}
557569

558-
// Root the context (and thus the delegate) for the lifetime of the window (see registry note above).
559-
RegisterTrackbarCallback(winName, trackbarName, contextHandle);
570+
// Root the context (and thus the delegate) for the lifetime of the window (see registry note above).
571+
RegisterTrackbarCallback(winName, trackbarName, contextHandle);
572+
}
560573
return ret;
561574
}
562575

‎src/OpenCvSharp/Fundamentals/CvObject.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ protected void SetSafeHandle(OpenCvSafeHandle handle)
8181
}
8282
}
8383

84+
/// <summary>
85+
/// Registers cleanup that runs after the owned native handle is released, including
86+
/// when cleanup is initiated by the SafeHandle finalizer.
87+
/// </summary>
88+
protected void SetPostReleaseAction(Action action)
89+
{
90+
var handle = safeHandle ?? throw new InvalidOperationException("A SafeHandle must be set first.");
91+
handle.SetPostReleaseAction(action);
92+
}
93+
8494
/// <summary>
8595
/// Releases the resources
8696
/// </summary>

‎src/OpenCvSharp/Fundamentals/CvPtrObject.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ protected CvPtrObject(IntPtr smartPtr, IntPtr rawPtr, Action<IntPtr> releaseSmar
2121
this.smartPtr = smartPtr;
2222
// The handle is the raw T* (so it marshals correctly to P/Invoke), but releasing it must
2323
// delete the smart pointer that actually owns the lifetime.
24-
SetSafeHandle(new OpenCvPtrSafeHandle(rawPtr, ownsHandle: true, releaseAction: _ => releaseSmartPtr(smartPtr)));
24+
SetSafeHandle(new OpenCvPtrSafeHandle(
25+
rawPtr,
26+
ownsHandle: true,
27+
releaseAction: _ => releaseSmartPtr(smartPtr),
28+
releaseNullHandle: smartPtr != IntPtr.Zero));
2529
}
2630

2731
/// <summary>

‎src/OpenCvSharp/Fundamentals/DisposableObjectHolder.cs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
namespace OpenCvSharp;
2+
3+
/// <summary>
4+
/// Atomically owns a replaceable disposable resource.
5+
/// </summary>
6+
internal sealed class DisposableObjectHolder<T> : IDisposable
7+
where T : class, IDisposable
8+
{
9+
private readonly object sync = new();
10+
private T? value;
11+
private bool disposed;
12+
13+
public void Replace(T? replacement)
14+
{
15+
T? previous;
16+
var rejectReplacement = false;
17+
lock (sync)
18+
{
19+
if (disposed)
20+
{
21+
previous = null;
22+
rejectReplacement = true;
23+
}
24+
else
25+
{
26+
previous = value;
27+
value = replacement;
28+
}
29+
}
30+
31+
if (rejectReplacement)
32+
{
33+
replacement?.Dispose();
34+
throw new ObjectDisposedException(GetType().FullName);
35+
}
36+
if (!ReferenceEquals(previous, replacement))
37+
previous?.Dispose();
38+
}
39+
40+
public void Dispose()
41+
{
42+
T? previous;
43+
lock (sync)
44+
{
45+
if (disposed)
46+
return;
47+
48+
disposed = true;
49+
previous = value;
50+
value = null;
51+
}
52+
53+
previous?.Dispose();
54+
}
55+
}

0 commit comments

Comments
 (0)