[zjxxzjwang]

Motivation

TableViewImpl.Close() iterates over all registered cancelReaders and calls
reader.Close() for each one, but it never calls cancelFunc() to cancel
the context passed to watchReaderForNewMessages.
Each watchReaderForNewMessages goroutine blocks on reader.Next(ctx).
Internally, reader.Close() calls consumer.closeWithCause() which invokes
wg.Wait() to wait for all partition consumer goroutines to finish. However,
because the context is never cancelled, reader.Next(ctx) may remain blocked
indefinitely — the goroutine cannot exit, wg.Wait() never completes, and
Close() hangs while holding readersMu. This is a deadlock.
Note that partitionUpdateCheck() already handles this correctly when removing
stale partitions — it calls cancelFunc() before reader.Close(). The Close()
method was simply missing the same treatment.

Modifications

• Call cancelReader.cancelFunc() before cancelReader.reader.Close() in
  TableViewImpl.Close(), consistent with the pattern in partitionUpdateCheck().

Verifying this change

• Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

• Added TestCloseCallsCancelFuncToAvoidDeadlock: creates a TableViewImpl with
  a mock reader whose Next() blocks on ctx.Done(), starts a watcher goroutine,
  then calls Close() and asserts it returns within 3 seconds. Without the fix,
  Close() deadlocks and the test times out.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): no
• The public API: no
• The schema: no
• The default values of configurations: no
• The wire protocol: no

Documentation

• Does this pull request introduce a new feature? no
• If yes, how is the feature documented? not applicable
• If a feature is not applicable for documentation, explain why?
  This is a bug fix for an internal deadlock, no user-facing feature is added.

[zjxxzjwang]

@crossoverJie
Hello, can you help me review this PR

[crossoverJie]

Could you rebase this onto the current master? @zjxxzjwang

[zjxxzjwang]

Could you rebase this onto the current master? @zjxxzjwang
@crossoverJie
I have rebased, please help me review.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

1 open issue

• As written, this mock does not reproduce the deadlock described in the PR: mockReader.Close() is… · New

What changed in this PR

Fixes a potential deadlock in TableViewImpl.Close() by canceling each reader’s context before closing the reader, and adds a regression test intended to ensure Close() returns promptly.

Changes:

• Cancel each reader’s context (cancelFunc()) before calling reader.Close() in TableViewImpl.Close().
• Add a unit test that attempts to reproduce and guard against the deadlock scenario.

File	Description
pulsar/​table_view_impl.go	Cancels reader contexts prior to closing readers to prevent Next(ctx) from blocking shutdown.
pulsar/​table_view_test.go	Adds a regression test and a mock reader intended to verify Close() doesn’t deadlock.

_{Review effort: Lite}

[Copilot]

🔵 Needs a closer look

The regression test passes even without the production cancellation call because the mock reader’s Close() is a no-op, so it must assert reader-context cancellation to detect the bug.

1 open issue

• As written, this mock does not reproduce the deadlock described in the PR: mockReader.Close() is…

_{Review effort: Balanced}

[zjxxzjwang]

@crossoverJie Hello, I have modified the test cases based on the feedback. Please review.