Skip to content

TUL/fix: prevent orphaned items - abort leaked request Context (backport #1352 to tul)#1354

Merged
milanmajchrak merged 2 commits into
customer/TULfrom
tul/be-abort-leaked-context
Jul 3, 2026
Merged

TUL/fix: prevent orphaned items - abort leaked request Context (backport #1352 to tul)#1354
milanmajchrak merged 2 commits into
customer/TULfrom
tul/be-abort-leaked-context

Conversation

@jr-rk

@jr-rk jr-rk commented Jul 2, 2026

Copy link
Copy Markdown

Problem

On this customer branch a submitted item can silently become orphanedowning_collection = NULL, in_archive = false, while its collection2item mapping and handle survive — so it vanishes from search/listings, has no breadcrumb, and "Edit Item" throws HTTP 500. Tracking issue: #1353. Source: #1352 (and original dc57ffded8 / #852).

Root cause

On the request error path a slow submission upload throws before context.commit(). DSpaceRequestContextFilter assigns its context local only after chain.doFilter(...), so on an exception the local stays null and its finally skips the abort → the Hibernate session leaks, still bound to the Tomcat thread holding a dirty, stale Item. A later request on that thread flushes the stale Item as a full-row UPDATE (Item has no @DynamicUpdate/@Version), reverting owning_collection → NULL / in_archive → false.

Change set

Backport of the single behavioral hunk from dc57ffded8 (as shipped to customer/zcu-data in #1352, commit ea6116ebec1): the outer StatelessAuthenticationFilter now wraps chain.doFilter(...) in try/finally and, in the finally, reads the Context from req.getAttribute(ContextUtil.DSPACE_CONTEXT) and abort()s it when still valid — cleaning up the leaked context the inner filter missed. No-op on the success path (request already committed → context no longer valid). One file, +17/-1; no new imports (Context/ContextUtil/log already present). Diagnostic/CI files from dc57ffded8 intentionally omitted.

Test evidence

# Cherry-pick applied cleanly (identical context) onto this branch.
# Added-hunk signature identical (b8c3482c8bdc) to the CI-green source commit ea6116ebec1 (#1352).
# Full module build + checkstyle + tests run in this PR's CI.

Risk & rollback

Very low: 17-line change, no schema/API change, success path is a no-op; identical to code already merged on dtq-dev / customer/zcu-data / customer/vsb-tuo / customer/lindat. Rollback = revert this single commit; no data migration.

Notes / assumptions

Behavioral-only backport. Defense-in-depth follow-ups are tracked separately in #1353 (null-local fix in DSpaceRequestContextFilter; @DynamicUpdate/@Version on Item/DSpaceObject; read-path 500 guard #1348).

…ackport dc57ffd / #852)

Backport the behavioral fix from dc57ffd ("Transaction bug - close
context in finally block", PR #852) which is on dtq-dev/customer/lindat
but missing from customer/zcu-data.

On the request error path a slow submission upload could leave its
Hibernate session bound to the Tomcat thread with a dirty, stale Item:
the upload throws before reaching context.commit(), and
DSpaceRequestContextFilter assigns its `context` local only AFTER
chain.doFilter, so on an exception the local stays null and its finally
skips the abort -> the session leaks. A later request on that thread then
flushes the stale Item as a full-row UPDATE (Item has no
@DynamicUpdate/@Version), reverting owning_collection->NULL and
in_archive->false while collection2item + handle survive -> the item
becomes an unsearchable orphan (e.g. handle 20.500.14592/107).

Wrap chain.doFilter in the outer StatelessAuthenticationFilter with a
try/finally that reads the Context from the request attribute and
abort()s it if still valid, cleaning up the leaked context the inner
filter missed. No-op on the normal success path.

Only the StatelessAuthenticationFilter hunk is ported; the diagnostic/CI
files in dc57ffd are omitted (they conflict and are not needed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit ea6116e)
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 183b6040-cef5-4a06-ad23-c058afc0d46f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI 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.

Pull request overview

Backports a defensive cleanup to prevent leaked request-scoped Context objects (and their Hibernate sessions) from persisting across requests, which can lead to lost updates and “orphaned” Items on error paths.

Changes:

  • Wraps chain.doFilter(req, res) in try/finally within StatelessAuthenticationFilter.
  • In finally, retrieves the request Context via ContextUtil.DSPACE_CONTEXT and aborts it if still valid, ensuring DB resources/transactions are released on exceptional request flows.

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

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@jr-rk jr-rk self-assigned this Jul 2, 2026
@jr-rk jr-rk requested a review from milanmajchrak July 2, 2026 17:12
@milanmajchrak milanmajchrak merged commit 0446ae5 into customer/TUL Jul 3, 2026
6 checks passed
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.

Backport: abort leaked request Context in StatelessAuthenticationFilter to all remaining customers (propagate #1352 / dc57ffded8)

3 participants