Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Tizen.Multimedia.AudioIO] Narrow Dispose() catch instead of swallowing all exceptions#7663

Open
JoonghyunCho wants to merge 1 commit into
main from
ai-task/issue-7610
Open

[Tizen.Multimedia.AudioIO] Narrow Dispose() catch instead of swallowing all exceptions #7663
JoonghyunCho wants to merge 1 commit into
main from
ai-task/issue-7610

Conversation

@JoonghyunCho

@JoonghyunCho JoonghyunCho commented May 24, 2026

Copy link
Copy Markdown
Member

Summary

Replaces the catch-all catch (Exception) { } swallow pattern in AudioCaptureBase.Dispose(bool) and AudioPlayback.Dispose(bool) with a narrower set of catch clauses, so unexpected failures inside Unprepare() show up in dlog instead of disappearing.

Changes

  • src/Tizen.Multimedia.AudioIO/AudioIO/AudioCapture.cs (Dispose(bool), around L131)
  • src/Tizen.Multimedia.AudioIO/AudioIO/AudioPlayback.cs (Dispose(bool), around L168)

For both files the previous block:

try
{
 Unprepare();
}
catch (Exception)
{
}

is now:

try
{
 Unprepare();
}
catch (ObjectDisposedException)
{
 // Expected during shutdown races.
}
catch (InvalidOperationException)
{
 // Expected if the underlying handle has already been torn down.
}
catch (Exception ex)
{
 Log.Error(GetType().Name, $"Unexpected error during Dispose: {ex}");
}

ObjectDisposedException is listed first because it derives from InvalidOperationException (C# requires the more-derived catch first).

Mode

Refactoring

Behavior & API Compatibility

Item Result
Dispose(bool) signature Unchanged
Throws from Dispose(bool) Still none (Log.Error is the only added side-effect for the unexpected case)
Normal teardown path Same (the two formerly-silent exception types stay silent)
Public API surface Unchanged
Tizen.Log dependency Already present in Tizen.Multimedia.AudioIO (see existing usage at AudioCapture.cs:474)

Verification

  • Build: dotnet build src/Tizen.Multimedia.AudioIO → 0 errors, 0 new warnings
  • Tests: N/A — no behavior change for the documented contract; existing AudioIO tests remain valid
  • Benchmark: skipped — change is on the dispose path, not a hot path, and only adds error-path logging. No sdb device available in this environment for runtime measurement.

Fixes #7610

gemini-code-assist[bot] reacted with eyes emoji
...allowing all exceptions
AudioCaptureBase.Dispose(bool) and AudioPlayback.Dispose(bool) wrapped
the cleanup call to Unprepare() in an unconditional 'catch (Exception)'
with an empty body. That hid every fault, including critical native
exceptions, with no log trace.
Narrow the silent path to the two cases that are normal during teardown
(ObjectDisposedException and InvalidOperationException) and route any
other exception to Log.Error so unexpected failures stay visible in
dlog. ObjectDisposedException is listed first because it derives from
InvalidOperationException.
Dispose() still does not rethrow, so external behavior and the
Dispose(bool) contract are unchanged. Public API is unchanged.
Fixes #7610 

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refines exception handling within the Dispose methods of AudioCapture and AudioPlayback. Specifically, it replaces a generic catch-all block with targeted handling for ObjectDisposedException and InvalidOperationException, which are now explicitly ignored with explanatory comments. Other unexpected exceptions are now logged for better observability. I have no feedback to provide.

Copy link
Copy Markdown
Member Author

🤖 [AI Review]

Reviewed — no findings.

Scope checked:

  • Exception ordering: ObjectDisposedException is listed before InvalidOperationException (correct, since the former derives from the latter — otherwise it would be unreachable)
  • Symmetry: identical catch ladder applied to both AudioCapture.Dispose(bool) and AudioPlayback.Dispose(bool)
  • No rethrow path introduced — Dispose(bool) contract preserved (cleanup still continues with Destroy(_handle) and _isDisposed = true)
  • Logged catch uses GetType().Name as tag and $ex (full exception with stack) — sufficient for dlog forensics
  • Public API surface unchanged (2 files, body-only edits inside protected virtual overrides)

No 🔴 critical issues, no 🟡 suggestions to flag.


Automated review — final merge decision rests with human reviewers.

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

Reviewers

@hsgwon hsgwon Awaiting requested review from hsgwon hsgwon is a code owner

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

1 participant

AltStyle によって変換されたページ (->オリジナル) /