Skip to content

Remove ComPortController finalizer and run deactivation on program shutdown#1427

Open
Copilot wants to merge 7 commits into
mainfrom
copilot/fix-comportcontroller-exception
Open

Remove ComPortController finalizer and run deactivation on program shutdown#1427
Copilot wants to merge 7 commits into
mainfrom
copilot/fix-comportcontroller-exception

Conversation

Copilot AI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

ComPortController could throw NullReferenceException during shutdown from its finalizer path. The controller was performing COM-port cleanup in ~ComPortController() even when no port instance existed.

  • Finalization cleanup moved out of GC path

    • Removed ~ComPortController() to eliminate shutdown-time finalizer exceptions and avoid cleanup work during object finalization.
  • Explicit shutdown behavior in Deactivate()

    • Kept cleanup in the explicit lifecycle path by unsubscribing SerialDataReceived in Deactivate().
    • Treated Port == null as already deactivated (return true) instead of a failure condition.
  • Program shutdown integration

    • Registered ComPortController to CrestronEnvironment.ProgramStatusEventHandler after successful initialization.
    • Calls Deactivate() when the program enters Stopping so COM port cleanup runs during system shutdown without relying on finalization.
    • Deactivate() now unsubscribes from the program status event and is guarded to be idempotent/thread-safe to prevent duplicate deactivation work.
  • Net effect

    • No COM port + shutdown now safely no-ops.
    • Existing unregister behavior is unchanged when a valid port exists.
    • COM port controllers now participate in the normal program stopping lifecycle.
public override bool Deactivate()
{
    lock (_deactivateLock)
    {
        if (_isDeactivated)
            return true;

        _isDeactivated = true;
    }

    CrestronEnvironment.ProgramStatusEventHandler -= CrestronEnvironment_ProgramStatusEventHandler;

    if (Port == null)
        return true;

    Port.SerialDataReceived -= Port_SerialDataReceived;
    return Port.UnRegister() == eDeviceRegistrationUnRegistrationResponse.Success;
}

Copilot AI changed the title [WIP] Fix ComPortController exception at shutdown in finalize method Remove ComPortController finalizer and make deactivation null-safe Jun 5, 2026
Copilot AI requested a review from ngenovese11 June 5, 2026 20:48
@ngenovese11

Copy link
Copy Markdown
Contributor

@copilot can we register to the Program shutting down event (may need to reference other classes for this) and call deactivate at shutdown

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can we register to the Program shutting down event (may need to reference other classes for this) and call deactivate at shutdown

Implemented in d4f1fec. ComPortController now subscribes to CrestronEnvironment.ProgramStatusEventHandler and calls Deactivate() on Stopping, with idempotent deactivation and cleanup/unsubscribe handling.

Copilot AI changed the title Remove ComPortController finalizer and make deactivation null-safe Remove ComPortController finalizer and run deactivation on program shutdown Jun 9, 2026
@ndorin ndorin marked this pull request as ready for review June 12, 2026 17:21
Copilot AI review requested due to automatic review settings June 12, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates ComPortController shutdown behavior to avoid COM-port cleanup running in the GC finalizer path and instead perform cleanup explicitly during program shutdown (eProgramStatusEventType.Stopping). The goal is to prevent shutdown-time exceptions and make deactivation safer and repeatable.

Changes:

  • Removed the ~ComPortController() finalizer-based cleanup.
  • Added a CrestronEnvironment.ProgramStatusEventHandler hook to call Deactivate() on program Stopping.
  • Made Deactivate() guarded with a lock/flag and treat Port == null as already deactivated, while also unsubscribing SerialDataReceived.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 62 to 66
RegisterAndConfigureComPort();
});

CrestronEnvironment.ProgramStatusEventHandler += CrestronEnvironment_ProgramStatusEventHandler;
}
Comment on lines +164 to +168
lock (_deactivateLock)
{
if (_isDeactivated)
return true;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ComPortController throws exception at shutdown in finalize method

4 participants