Skip to content

[Quality] 错误处理: Critical Issues - assert for error handling, silent failures, resource leaks #12

@newtontech

Description

@newtontech

🚨 Critical Error Handling Issues

This issue documents actual error handling vulnerabilities in DAM3000M.py that can cause silent failures, resource leaks, and production crashes.


Issue 1: Using assert for Runtime Error Handling (CRITICAL)

File: DAM3000M.py:134, 135, 144, 147, 169, 172, 207, 213, 218, 220

Problematic Code (lines 134-135):

assert handle not in (-1,None, 0) ,"Failed to create device."
assert DAM3000M_InitDevice(handle, self.baud_rate, 8, 0, 0x00, 200, 0),"Failed to initialize device."

Why This Is Dangerous:

  1. Assertions can be disabled - Running with python -O or PYTHONOPTIMIZE=1 disables ALL assertions, causing these critical error checks to be completely bypassed
  2. Wrong exception type - AssertionError is for debugging invariants, not runtime errors
  3. Hardware operations fail silently - Device initialization failures are treated as programming bugs, not expected runtime conditions

Impact:

  • Production hardware control code with disabled assertions will continue execution after device creation/init failures
  • Undefined behavior when accessing invalid device handles
  • Silent data corruption in analog I/O operations

Recommended Fix:

class DeviceError(Exception):
    """Custom exception for device operations."""
    pass

class DAMDevice:
    def _get_handle(self):
        if self.com_id not in self.handles:
            handle = DAM3000M_CreateDevice(self.com_id)
            if handle in (-1, None, 0):
                raise DeviceError(f"Failed to create device on COM{self.com_id}")
            
            success = DAM3000M_InitDevice(handle, self.baud_rate, 8, 0, 0x00, 200, 0)
            if not success:
                raise DeviceError(f"Failed to initialize device on COM{self.com_id}")
            
            self.handles[self.com_id] = handle
        return self.handles[self.com_id]

Issue 2: Silent DLL Loading Failure (CRITICAL)

File: DAM3000M.py:75-76

Problematic Code:

dll_path = "./DAM3000M_64.dll"
dam3000m = ctypes.WinDLL(dll_path)

Why This Is Dangerous:

  1. No try/except block - crashes entire application with unhandled OSError if DLL is missing
  2. Hardcoded relative path - breaks if script is run from different directory
  3. No error message to help user diagnose the problem
  4. Windows-specific code without platform checks

Impact:

  • Application crashes with cryptic traceback on missing DLL
  • No actionable error message for end users
  • Path resolution fails when executed outside script directory

Recommended Fix:

import os
import platform

class ConfigurationError(Exception):
    pass

# Platform check
if platform.system() != 'Windows':
    raise ConfigurationError("This module requires Windows (uses WinDLL)")

# DLL loading with proper error handling
dll_path = os.path.join(os.path.dirname(__file__), "DAM3000M_64.dll")
if not os.path.exists(dll_path):
    raise ConfigurationError(
        f"Required DLL not found: {dll_path}\n"
        "Please ensure DAM3000M_64.dll is in the same directory as this script."
    )

try:
    dam3000m = ctypes.WinDLL(dll_path)
except OSError as e:
    raise ConfigurationError(f"Failed to load DLL: {e}") from e

Issue 3: Broken Context Manager Implementation (HIGH)

File: DAM3000M.py:146-147

Problematic Code:

def __exit__(self):
    assert DAM3000M_ReleaseDevice(self.handle),"Failed to release device."

Why This Is Dangerous:

  1. Missing __enter__ method - DAMDevice defines __exit__ but no __enter__, making with statements fail
  2. Wrong __exit__ signature - Should be __exit__(self, exc_type, exc_val, exc_tb)
  3. Uses assert instead of proper error handling (see Issue 1)

Impact:

  • with DAMDevice(...) as device: raises AttributeError: __enter__
  • Cannot use proper resource management patterns
  • Resource leaks when exceptions occur during device use

Recommended Fix:

class DAMDevice:
    def __enter__(self):
        return self
    
    def __exit__(self, exc_type, exc_val, exc_tb):
        """Context manager exit - always attempt cleanup."""
        success = DAM3000M_ReleaseDevice(self.handle)
        if not success:
            # Log error but don't raise during exception handling
            import logging
            logging.error(f"Failed to release device handle {self.handle}")
        return False  # Don't suppress exceptions

Issue 4: Uninitialized Instance Attribute (HIGH)

File: DAM3000M.py:167, 173

Problematic Code:

def set_analog_output(self, lChannel:int, value:float):
    range_bottom, range_top = self.RangeModes[self.ChannelModes[lChannel]]  # ChannelModes not defined!
    ...

def set_output_range_mode(self, lChannel:int, range_mode:int):
    ...
    self.ChannelModes[lChannel]=range_mode  # First assignment

Why This Is Dangerous:

  1. self.ChannelModes is accessed in set_analog_output before initialization
  2. Calling set_analog_output before set_output_range_mode raises AttributeError
  3. Initialization in __init__ only calls set_output_range_mode for default modes

Impact:

  • AttributeError: 'DAM3060V' object has no attribute 'ChannelModes' if called out of order
  • Race conditions in multi-threaded usage

Recommended Fix:

class DAM3060V(DAMDevice):
    def __init__(self, com_id, baud_rate, device_id):
        self.ChannelModes = {}  # Initialize before super().__init__
        super().__init__(com_id, baud_rate, device_id)
        for lChannel in range(self.ChannelNum):
            self.set_output_range_mode(lChannel, self.ModeList[lChannel])

Issue 5: Resource Leak on Partial Initialization Failure (HIGH)

File: DAM3000M.py:131-137

Problematic Code:

def _get_handle(self):
    if self.com_id not in self.handles:
        handle=DAM3000M_CreateDevice(self.com_id)
        assert handle not in (-1,None, 0) ,"Failed to create device."
        assert DAM3000M_InitDevice(handle, self.baud_rate, 8, 0, 0x00, 200, 0),"Failed to initialize device."
        self.handles[self.com_id]=handle
    return self.handles[self.com_id]

Why This Is Dangerous:

  1. If device creation succeeds but initialization fails, the handle is never released
  2. Handle leak on DAM3000M_InitDevice failure
  3. No cleanup path for partially initialized resources

Impact:

  • Handle exhaustion after repeated initialization attempts
  • System resource leak requiring process restart

Recommended Fix:

def _get_handle(self):
    if self.com_id not in self.handles:
        handle = DAM3000M_CreateDevice(self.com_id)
        if handle in (-1, None, 0):
            raise DeviceError(f"Failed to create device on COM{self.com_id}")
        
        try:
            success = DAM3000M_InitDevice(handle, self.baud_rate, 8, 0, 0x00, 200, 0)
            if not success:
                raise DeviceError(f"Failed to initialize device on COM{self.com_id}")
            self.handles[self.com_id] = handle
        except Exception:
            # Cleanup on failure
            DAM3000M_ReleaseDevice(handle)
            raise
    return self.handles[self.com_id]

Issue 6: Missing Error Context from DLL Functions

File: DAM3000M.py (multiple locations)

Problematic Code (line 172):

assert DAM3000M_WriteSingleReg(self.handle,self.device_id,self.RangeAddr+lChannel,range_mode),f'Failed to set range mode for  {self.handle=}, {self.device_id =}, {self.RangeAddr+lChannel =}, {range_mode =}.'

Why This Is Dangerous:

  1. Error message includes parameters but not why the operation failed
  2. No errno/GetLastError retrieval from Windows API
  3. Users cannot diagnose hardware communication issues

Recommended Fix:

def _check_result(self, success: bool, operation: str):
    """Check DLL operation result and raise with context."""
    if not success:
        import ctypes
        error_code = ctypes.get_last_error()
        raise DeviceError(
            f"{operation} failed. "
            f"Windows error code: {error_code} "
            f"(0x{error_code:08X})"
        )

# Usage:
result = DAM3000M_WriteSingleReg(...)
self._check_result(result, f"Set range mode for channel {lChannel}")

Summary

Issue Severity Impact
assert for runtime errors CRITICAL Silent failures in production
Silent DLL loading CRITICAL Unhandled crashes
Broken context manager HIGH Resource leaks, unusable with
Uninitialized attribute HIGH AttributeError at runtime
Resource leak HIGH Handle exhaustion
Missing error context MEDIUM Undiagnosable failures

Recommended Priority

  • 高 - 影响系统稳定性/安全性 (Issues 1, 2)
  • 中 - 影响代码质量 (Issues 3, 4, 5, 6)

Environment

  • Python version: Any (issues affect all versions)
  • Platform: Windows (DLL dependency)
  • Hardware: DAM3000M series devices

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions