Skip to content

[XMLBEANS-676] avoid NamespaceContext thread-local leaks - #125

Merged
pjfanning merged 1 commit into
apache:trunkfrom
pjfanning:XMLBEANS-676-namespace-context-leak
Oct 4, 2026
Merged

pjfanning merged 1 commit into
apache:trunkfrom
pjfanning:XMLBEANS-676-namespace-context-leak

Conversation

@pjfanning

Copy link
Copy Markdown
Member

Fixes https://issues.apache.org/jira/browse/XMLBEANS-676

Ways the NamespaceContext thread-local could be left holding memory:

  • getCurrent() created and stored a stack when none existed. JavaQNameHolder/JavaNotationHolderEx call it outside any push/pop, so the stack was never removed.
  • NamespaceContextStack.pop() set current and shrank the stack in two separate steps. An exception between them (e.g. from Thread.stop) left them out of step, so the stack never emptied and the thread-local kept a stale current (a TypeStore holding the whole document's store).
  • SchemaParticleImpl and SchemaLocalAttributeImpl called push inside the try, so a failed push (e.g. OOM) still ran pop and removed the caller's frame. On an empty stack pop threw IndexOutOfBoundsException, which hid the original error.

Changes:

  • getCurrent() returns null without creating state
  • pop() uses a single stack.remove(...); an unbalanced pop cleans up instead of throwing
  • ThreadLocal.remove() instead of set(null) when the stack empties
  • push moved before the try in the two schema classes
  • new NamespaceContextTest

This can't protect against Thread.stop landing between a caller's push and its try. Callers in that situation should use ThreadLocalUtil.clearAllThreadLocals().

🤖 Generated with Claude Code

- getCurrent() no longer creates thread-local state that is never removed
- pop() updates current and the stack in one step and tolerates an
  unbalanced pop instead of throwing (which masked the original exception)
- remove() the thread-local when the stack empties
- push before the try in SchemaParticleImpl/SchemaLocalAttributeImpl so a
  failed push does not pop the caller's frame

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pjfanning

Copy link
Copy Markdown
Member Author

None of the problems fixed here are recent regressions. They all go back to the original code:

Issue Introduced
getCurrent() creates a stack that is never removed b3b7d1e (2005)
two-step pop() (get then remove) initial checkin f6f24b7 (2003). 7fe6c92 (2022) only added generics
pop() throws on an empty stack and uses set(null) rather than remove() 515b0cb (2005)
push inside the try in SchemaParticleImpl / SchemaLocalAttributeImpl initial checkin f6f24b7 (2003)

XMLBEANS-502 (d1a3f12) added ThreadLocalUtil.clearAllThreadLocals() as a manual workaround, and left these code paths alone. The leak probably shows up now because the POI regression run reuses threads across millions of documents, uses Thread.stop and catches OOMs.

@pjfanning
pjfanning merged commit e1e57ca into apache:trunk Oct 4, 2026
3 checks passed
@pjfanning
pjfanning deleted the XMLBEANS-676-namespace-context-leak branch October 4, 2026 09:54
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.

1 participant