Repository navigation
feat(libSceUserService): implement event callback delivery - #14848
Conversation
Progress report (bbc1388 → 4977956)📈 System libraries: 30.07% (+0.01%, +5 implemented functions) ✅ 5 implemented
Changed libraries (implemented exports)
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — changes requested
The callback lifetime implementation is internally coherent, but the firmware behavior claims lack the source locations required by the repository’s reverse-engineering guidance.
What I ran
Reviewed head 8f848048b1c361998cb6888324181a8d5cad4e4e against base e0caf1360035286bd22b9f2a3030e97b880ffb28; the PR itself merges cleanly against that base. Environment: Debian 12, Linux 6.1, Python 3.11.2, GCC 12.2. No local build or CTest was run. The author reports a Windows build and lifecycle test pass; those results were not independently reproduced.
git diff --check e0caf1360035286bd22b9f2a3030e97b880ffb28...8f848048b1c361998cb6888324181a8d5cad4e4e
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14848
python3 tools/check_conventions.py --base e0caf1360035286bd22b9f2a3030e97b880ffb28 --head 8f848048b1c361998cb6888324181a8d5cad4e4e --title 'fix(libSceUserService): make event callbacks reentrant' --body /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14848-8f848048b1c3/pr-body.md
python3 tools/pr_abi_audit.py --base e0caf1360035286bd22b9f2a3030e97b880ffb28 --head 8f848048b1c361998cb6888324181a8d5cad4e4e --json
python3 tools/pr_stack_check.py 14848 --repo boykopovar/AnyPS5 --output-dir /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14848-8f848048b1c3/stack
python3 tools/pr_test_quality.py --root /marc/AnyPS5 --json core/libs/tests/GuestUserServiceLifecycle.cpp
git grep -n -E 'GuestUserServiceLifecycle\.cpp|guest_user_service_lifecycle' 8f848048b1c361998cb6888324181a8d5cad4e4e -- '*CMakeLists.txt' '*.cmake'
gh pr checks 14848 --repo boykopovar/AnyPS5Results
| Check | Result | Note |
|---|---|---|
| Diff and conventions | passed | No whitespace errors; conventions passed. |
| Export audit | passed | No lexical ABI findings. |
| Test registration | passed | Lifecycle test is registered in CMake. |
| Stack | failed | Declared dependency heads are not included; two dependency PRs conflict with current main. |
| Hosted checks | not run | Windows remains pending; portability and report passed. |
| Build and CTest here | not run | Author reports a Windows build and focused test pass. |
Precheck and hosted-check details
The precheck confirmed the head and base were unchanged. Its isolated snapshot lacked the workspace-only ABI and test-quality helper scripts. The standalone ABI audit found zero findings. The test-quality heuristic warned that the C++ test might be unregistered; direct CMake inspection confirmed registration at core/libs/prx/libSceUserService/CMakeLists.txt:31-36.
The stack report says #14848 itself merges cleanly against the current base, but its declared chain is #9345 → #14212 → #12752 → #14829 → #14848. The current #9345 and #14212 heads conflict with main, and the current #14829 head is not included in #14848. These are dependency-state facts, not a conflict in #14848’s own diff.
gh pr checks 14848 --repo boykopovar/AnyPS5:
windows pending
linux skipping
register-context skipping
relinker skipping
relinker-asan skipping
write-watch skipping
conventions pass
eligibility pass
intake pass
labels pass
library-portability pass
report pass
Negative control
| Assertion | Change to code under test | Did the new test fail? |
|---|---|---|
| Unregister and termination wait for in-flight callbacks | No local mutation or test execution was performed. | Could not be exercised here. |
Findings
Blocking
The PR and debt entry say firmware 13.60 stores callback/user-data pairs and sends 8-byte login/logout events, but they do not identify the relevant libSceUserService.sprx addresses or explain what each observed site establishes. The reverse-engineering guide requires the module and firmware version, the source locations, what each location shows, and what remains unverified. The tests demonstrate the behavior of this implementation; they do not establish the firmware contract. Please add the source locations and distinguish firmware-observed behavior from host-side lifecycle choices such as waiting for callbacks to drain. If null-callback handling or duplicate-registration behavior was not established from firmware, label it as unverified rather than implying the test confirms it.
Non-blocking
The dependency stack needs reconciliation before landing: #9345 and #14212 currently conflict with main, and #14829’s current head is not included in this branch. The PR lists these dependencies, so I am not asking for an unrelated rebase now.
Sources
- Repository guidance for citing firmware and title evidence
- Contribution rule on signatures and behavior inferred from call sites
- Firmware behavior and lifecycle claims in the changed debt entry
- Callback registration, dispatch, unregister and termination at this head
- Lifecycle assertions at this head
Not verified
No local build, CTest, firmware execution, console behavior, or source-module inspection was performed. The Windows hosted job is pending; Linux and specialized jobs were skipped. The author-reported Windows results were not reproduced here.
To get this to a good state
Please add the firmware source locations and identify which claims they support, while marking any host-only drain behavior or unverified error cases as such. When the source and/or code changes, I will rerun the precheck, diff, ABI, stack and test-registration checks and review the new head; the pending hosted jobs should also be refreshed for that SHA.
|
Thanks for this. A few things before it can go in:
On the tests: |
|
Implemented the five requested corrections. Delivery now runs on a dedicated host worker, so a blocked title callback does not block the presentation/input caller. Cross-callback delivery is serialized, eliminating the cyclic drain wait. Unregister marks entries inactive but retains them until in-flight calls finish, so termination and a second unregister still account for them. The synchronized regressions include both notification producers and unregister/termination during a blocked callback; 30 consecutive lifecycle runs passed. The exact registration, unregister, native worker and 8-byte message locations are now cited, including the asymmetric null cases. Host lifecycle choices are identified separately. I corrected the title to describe newly implemented callback delivery and removed the unrelated dependency chain. AI-assisted: yes. |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
Co-Authored-By: Claude <noreply@anthropic.com>
585a0f0 to
4977956
Compare
|
Checked the callback registration/unregister/drain logic, the dedicated delivery worker, the tests (they need the new exports, which were throwing stubs on main) and the debt entries. Rebased onto current |
What
Implement event callback registration, asynchronous delivery and unregistration. One host worker invokes callbacks outside service locks; controller/presentation callers only enqueue events. Removed registrations remain in drain accounting until their active call finishes, including overlapping unregister and termination. Delivery is serialized, preventing mutual cross-callback drain waits. Self-unregister and removal after snapshot remain supported.
Firmware 13.60 libSceUserService.sprx evidence: registration 0x34d0 establishes unique pairs, null-success and the initialization check; unregister 0x35f0 establishes null rejection and absent-pair success. Initialization helper 0x1930 creates SceUserServiceEvent at 0x2380, which drains messages through 0x1e10. The latter requests 8-byte events and invokes the callback with the event pointer and stored user data, releasing the callback-list lock first. Shell wrappers are at 0x28a0 and 0x24d0. The debt entry records the module digest and separates these static observations from host snapshots, drain accounting and scheduling choices. Console behavior was not measured.
The shell callback table, IPMI backend and event sources beyond login/logout remain unavailable. This implementation is independent of the settings, group, user-name and debt-formatting PRs; shared source/debt files may need merge-order reconciliation.
Tested
Windows x86_64, WinLibs GCC, CMake/Ninja Release: built libSceUserService and guest_user_service_lifecycle_tests. Lifecycle CTest passed for 30 consecutive runs. Coverage includes two synchronized notification producers with cross-unregister, off-caller-thread blocking delivery, self-removal, removal after snapshot, inactive registrations retained during concurrent unregister/termination, duplicate/null registrations and calls before initialization. Drain tests synchronize on registration/lifecycle state rather than a sleep. The test compiles the production source directly to inspect that private state. Public-header audit and conventions passed. New-head hosted checks pending. No console or new Linux runtime run.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)