Let super().end_scan() run even if the 2BM optional steps fail - #188
Open
xmap wants to merge 1 commit into
Open
Conversation
end_scan() here does two things beyond the base class's own cleanup: appends a web camera frame to the scan file, and copies the finished file to the data analysis computer. Neither is guarded, so an exception in either one stops super().end_scan() from ever running, which means ScanStatus is never set to its final value, StartScan is never put back to 0, and scan_is_running is never cleared. add_theta(), called a few lines earlier in this same method, already wraps its own hdf5 access in a bare except that logs and prints a traceback, on the reasoning that a problem writing one auxiliary dataset should not stop the rest of end_scan() from completing. The webcam frame and the file transfer are exactly the same shape of problem: optional, best-effort steps that write into a scan file which may already be in a bad state. This gives each block the same guard, so a failure in either is logged and super().end_scan() still runs. Untested locally: this repo has no test coverage of end_scan(), and building an EPICS harness for it is out of scope here. To reproduce, make either block raise (for example, point the credentials file at a missing path, or force create_dataset to fail against a truncated file) and confirm super().end_scan() still runs and StartScan reaches 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to the fix in
fix/end-scan-must-not-wedge-startscan, branchedseparately from
masterso the two can be reviewed and mergedindependently. That PR stops a raise inside
end_scan()from killing thescan thread outright. This one removes the most concrete cause of such a
raise at 2-BM: the two unguarded, best-effort steps this override adds
before calling
super().end_scan().Mechanism.
TomoScan2BM.end_scan()does two things beyond the baseclass's own cleanup: it appends a web camera frame to the scan file with
fid.create_dataset('exchange/web_camera_frame', data=frame), and itcopies the finished file to the analysis computer with
dm.fdt_scp()ordm.scp(). Neither block is guarded. If either raises,super().end_scan()never runs, so
ScanStatusis never set to its final value,StartScanisnever put back to 0, and
scan_is_runningis never cleared. This isexactly what happened on 2026-08-20: a full disk truncated the scan file,
create_datasetraisedValueError: Unable to synchronously create dataset (address of object past end of allocation), and the scan thread diedbefore
super().end_scan()ran.add_theta(), called a few lines earlier in this same method, alreadywraps its own hdf5 access in a bare
exceptthat logs an error and printsa traceback, on the same reasoning: a problem writing one auxiliary dataset
should not stop the rest of
end_scan()from completing.add_theta()survived the identical failure on the identical file during the incident.
This gives the webcam-frame block and the data-transfer block the same
guard, each in its own
try/except, matchingadd_theta()'s style and loglevel (
log.errorplustraceback.print_exc(file=sys.stdout)).Scope. Only
tomoscan/tomoscan_2bm.py, only theend_scan()method.The two blocks are each wrapped in a
try/except; nothing inside eitherblock changes, and
super().end_scan()is unchanged. No other method istouched. The re-indentation needed to wrap the blocks also strips some
trailing whitespace on lines inside them; that is incidental to adding the
guards, not a separate cleanup pass.
Testing. This repo has no test coverage of
end_scan(), and buildingan EPICS harness for it is out of scope for this change. To reproduce:
make either block raise (for example, point the credentials file at a
missing path, or force
create_datasetto fail against a truncated file)and confirm
super().end_scan()still runs andStartScanreaches 0.