Skip to content

[#162] Stop the XML search at the handler's first false and call the handler outside the file lock - #204

Open
maximthomas wants to merge 4 commits into
OpenIdentityPlatform:masterfrom
maximthomas:issues/162-xml-search-handler
Open

maximthomas wants to merge 4 commits into
OpenIdentityPlatform:masterfrom
maximthomas:issues/162-xml-search-handler

Conversation

@maximthomas

Copy link
Copy Markdown
Contributor

Closes #162.

XMLConnector.executeQuery passed every result to the handler inside synchronized (getLock()) and ignored the false it returned.

  • The handler kept being called. With producerBufferSize = 0, a handler that returns false on the first object was called for every object in the file. The loop now stops at the first false.
  • The handler ran under the per-file lock. A slow handler held up every create, update, delete and search on the file. A handler that changes the same file never returned at all: with the default buffer of 100, the connector's thread holds the lock while it waits for room in the buffer, and the caller's thread, inside the handler, waits for the lock. A search over 150 accounts whose handler calls facade.update on the first one hangs at 247458d and completes in about a second with this change. The search now builds its result list under the lock and calls the handler after leaving it. The list holds finished ConnectorObjects with copied values and no DOM references.
  • Two connectors on one file could use two locks. lockMap was a static HashMap read and written under each connector instance's own monitor. It was keyed by getAbsolutePath(), so dir/./f.xml and dir/f.xml got different locks while they shared one cached handler, which is keyed by getCanonicalPath(). lockMap and getLock() are gone. Each connector now locks on the file's shared ConcurrentXMLHandler from XMLHandlerCache: there is one per canonical path, created under XMLConnector.class, so every connector on the file locks the same object.

create, update and delete stay under the lock. The log line Query returned {0} object(s) now counts the objects passed to the handler.

Tests

New in XMLConnectorTests:

  • searchStopsAtTheFirstFalse: a facade with setProducerBufferSize(0) searches five accounts with a handler that returns false. The handler is called once; it was called 5 times before.
  • searchHandlerDoesNotHoldUpAChangeOnTheSameFile: while one connector's search handler blocks, a second connector on the same file creates an account within 10 s. Before, the create timed out.
  • searchesOnOneFileStillTakeTurnsBuildingResults: two connectors on one file, one of them through a ./ path. The second search waits while the first builds its results. It fails with the old absolute-path key, with a lock per connector instance, and with no lock in executeQuery.
  • searchOnOneFileDoesNotHoldUpAChangeOnAnotherFile: a blocked search on one file does not hold up a create on another. It fails if all files share one lock.

The last two tests put a stub handler in XMLHandlerCache. The unsupported methods of the existing BlockingHandler stub moved to a shared StubHandler.

Not changed

  • The search still builds the whole result list before it calls the handler. Streaming the results would hold the handler's read lock while the caller's handler runs.
  • authenticate takes the handler's read lock but not the per-file lock, as before.

@maximthomas maximthomas added bug Something isn't working java Pull requests that update java code tests Test additions or fixes connector:xml XML connector concurrency Races, locking and thread-safety fixes labels Oct 23, 2026
… file lock before asserting it waits

searchesOnOneFileStillTakeTurnsBuildingResults asserted the wait with a 500 ms negative await, which stayed green on a no-lock executeQuery when the second thread started late. It now waits until that thread is BLOCKED on the file's shared handler, or has reached the stub, and then asserts. Also move the init() comment back above the call it explains.
@maximthomas
maximthomas marked this pull request as ready for review October 23, 2026 18:21
@maximthomas
maximthomas requested a review from vharseko October 23, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working concurrency Races, locking and thread-safety fixes connector:xml XML connector java Pull requests that update java code tests Test additions or fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XML connector: search ignores the handler's false and calls the handler under the per-file lock

1 participant