Skip to content

Commit 4cecdff

Browse files
rjshadehkpeprah
authored andcommitted
Protect against multiple calls to JLINKARM_Close (#65)
* Protect against multiple calls to JLINKARM_Close With the latest JLink DLLs (tested with 6.56b), calling jlink.close() twice can crash. This is a problem because there is an uncoditional call to close() inside jlink.open(). Any code that attempts to cycle a jlink connection by calling .close() followed by .open() therefore hits a double close and the corresponding crash. Crash from inside JLINKARM_Close looks like: `double free or corruption (out)` * Indentation * Reference counting .open() calls * Explicitly set the refcount in test * Remove blank lines to placate pycodestyle * Test setup ensures .open() called in context manager
1 parent b754a48 commit 4cecdff

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

pylink/jlink.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,10 @@ def __init__(self, lib=None, log=None, detailed_log=None, error=None, warn=None,
275275
self._lock = None
276276
self._device = None
277277

278+
# Track the number of .open() calls to avoid multiple calls to
279+
# JLINKARM_Close, which can cause a crash.
280+
self._open_refcount = 0
281+
278282
# Bind Types for function calls.
279283
self._dll.JLINKARM_OpenEx.restype = ctypes.POINTER(ctypes.c_char)
280284
self._dll.JLINKARM_GetCompileDateTime.restype = ctypes.POINTER(ctypes.c_char)
@@ -639,6 +643,9 @@ def open(self, serial_no=None, ip_addr=None):
639643
TypeError: if ``serial_no`` is present, but not ``int`` coercible.
640644
AttributeError: if ``serial_no`` and ``ip_addr`` are both ``None``.
641645
"""
646+
if self._open_refcount > 0:
647+
self._open_refcount += 1
648+
return None
642649

643650
# For some reason, the J-Link driver complains if this isn't called
644651
# first (may have something to do with it trying to establish a
@@ -692,6 +699,7 @@ def open(self, serial_no=None, ip_addr=None):
692699
func = enums.JLinkFunctions.UNSECURE_HOOK_PROTOTYPE(unsecure_hook)
693700
self._dll.JLINK_SetHookUnsecureDialog(func)
694701

702+
self._open_refcount = 1
695703
return None
696704

697705
def open_tunnel(self, serial_no, port=19020):
@@ -719,6 +727,14 @@ def close(self):
719727
Raises:
720728
JLinkException: if there is no connected JLink.
721729
"""
730+
if self._open_refcount == 0:
731+
# Do nothing if .open() has not been called.
732+
return None
733+
734+
self._open_refcount -= 1
735+
if self._open_refcount > 0:
736+
return None
737+
722738
self._dll.JLINKARM_Close()
723739

724740
if self._lock is not None:

tests/unit/test_jlink.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -818,8 +818,7 @@ def test_jlink_open_serial_number_context_manager_manual(self):
818818
jl.open(serial_no=123456789)
819819
self.dll.JLINKARM_OpenEx.assert_called()
820820
self.assertEqual(1, self.dll.JLINKARM_OpenEx.call_count)
821-
# 2 instead of 1 because jlink.close() is also called from jlink.open().
822-
self.assertEqual(2, self.dll.JLINKARM_Close.call_count)
821+
self.assertEqual(1, self.dll.JLINKARM_Close.call_count)
823822

824823
@mock.patch('pylink.jlock.JLock', new=mock.Mock())
825824
def test_jlink_open_dll_failed(self):
@@ -922,8 +921,21 @@ def test_jlink_close(self):
922921
Returns:
923922
``None``
924923
"""
924+
# close() does nothing if there has been no open() call first.
925+
self.jlink.close()
926+
self.assertEqual(0, self.dll.JLINKARM_Close.call_count)
927+
928+
# close() decrements the refcount if open() has been called multiple times.
929+
self.jlink._open_refcount = 5
930+
self.jlink.close()
931+
self.assertEqual(0, self.dll.JLINKARM_Close.call_count)
932+
self.assertEqual(4, self.jlink._open_refcount)
933+
934+
# close() calls the DLL close method when refcount is exhausted.
935+
self.jlink._open_refcount = 1
925936
self.jlink.close()
926937
self.assertEqual(1, self.dll.JLINKARM_Close.call_count)
938+
self.assertEqual(0, self.jlink._open_refcount)
927939

928940
def test_jlink_close_context_manager(self):
929941
"""Tests the J-Link ``close()`` method using context manager.
@@ -934,10 +946,11 @@ def test_jlink_close_context_manager(self):
934946
Returns:
935947
``None``
936948
"""
937-
# Use open_tunnel=None to avoid implicitly opening the connection.
938-
with jlink.JLink(self.lib, open_tunnel=None):
939-
self.dll.JLINKARM_OpenEx.assert_not_called()
949+
self.dll.JLINKARM_OpenEx.return_value = 0
950+
with jlink.JLink(self.lib, ip_addr='127.0.0.1:80') as jl:
951+
self.assertTrue(jl.opened())
940952
self.dll.JLINKARM_Close.assert_not_called()
953+
941954
# .close() is first called when exiting the context manager
942955
# Depending on the system - GC operation, it can also already be
943956
# called from __del__ when the object is garbage collected.

0 commit comments

Comments
 (0)