Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Segmentation fault in atropos() if more than 9 temporary files are created #399

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

phistep
Copy link
Contributor

@phistep phistep commented Sep 14, 2022

If the input list i for multivariate segmentation contains more than 9, that
is a double-digit number, of items, an error in filename format string
generation leads to an additional 0 being padded to the 10th filename during
creation.

ls /tmp/ants*
antsr6rhpmv2eprob01.nii.gz   antsr6rhpmv2eprob04.nii.gz  antsr6rhpmv2eprob08.nii.gz  antsr6rhpmv2eprob13.nii.gz
antsr6rhpmv2eprob010.nii.gz  antsr6rhpmv2eprob05.nii.gz  antsr6rhpmv2eprob09.nii.gz  antsr6rhpmv2eprob14.nii.gz
antsr6rhpmv2eprob02.nii.gz   antsr6rhpmv2eprob06.nii.gz  antsr6rhpmv2eprob11.nii.gz  antsr6rhpmv2eprob15.nii.gz
antsr6rhpmv2eprob03.nii.gz   antsr6rhpmv2eprob07.nii.gz  antsr6rhpmv2eprob12.nii.gz  antsr6rhpmv2eprob16.nii.gz

But the ANTs C extension expects the filename to only have two digits, not
three: antsr6rhpmv2eprob10.nii.gz. Upon failing to find the 10th image, it
segmentation faults:

test.py  file /tmp/antsr43ts_w23prob10.nii.gz does not exist . 
Fatal Python error: Segmentation fault
Current thread 0x00007fb238a1c740 (most recent call first):
  File "/usr/local/lib/python3.8/dist-packages/ants/segmentation/atropos.py", line 139 in atropos
  File "/src/tests/test.py", line 44 in test.py
  File "/usr/lib/python3.8/unittest/case.py", line 633 in _callTestMethod
  File "/usr/lib/python3.8/unittest/case.py", line 676 in run
  File "/usr/lib/python3.8/unittest/case.py", line 736 in __call__
  File "/usr/local/lib/python3.8/dist-packages/_pytest/unittest.py", line 330 in runtest
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 166 in pytest_runtest_call
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_callers.py", line 39 in _multicall
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_manager.py", line 80 in _hookexec
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_hooks.py", line 265 in __call__
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 259 in <lambda>
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 338 in from_call
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 258 in call_runtest_hook
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 219 in call_and_report
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 130 in runtestprotocol
  File "/usr/local/lib/python3.8/dist-packages/_pytest/runner.py", line 111 in pytest_runtest_protocol
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_callers.py", line 39 in _multicall
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_manager.py", line 80 in _hookexec
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_hooks.py", line 265 in __call__
  File "/usr/local/lib/python3.8/dist-packages/_pytest/main.py", line 347 in pytest_runtestloop
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_callers.py", line 39 in _multicall
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_manager.py", line 80 in _hookexec
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_hooks.py", line 265 in __call__
  File "/usr/local/lib/python3.8/dist-packages/_pytest/main.py", line 322 in _main
  File "/usr/local/lib/python3.8/dist-packages/_pytest/main.py", line 268 in wrap_session
  File "/usr/local/lib/python3.8/dist-packages/_pytest/main.py", line 315 in pytest_cmdline_main
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_callers.py", line 39 in _multicall
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_manager.py", line 80 in _hookexec
  File "/usr/local/lib/python3.8/dist-packages/pluggy/_hooks.py", line 265 in __call__
  File "/usr/local/lib/python3.8/dist-packages/_pytest/config/__init__.py", line 164 in main
  File "/usr/local/lib/python3.8/dist-packages/_pytest/config/__init__.py", line 187 in console_main
  File "/usr/local/bin/pytest", line 8 in <module>

This is caused by a minor off-by-one error and can be fixed as shown in the
patch included in this PR.

Unfortunately, I cannot provide a MWE at the time, but if it is required, I
will try to create one.

I am new to contributing to this project, but the guide did not explain your
usual bug workflow. How do I proceed from here?

  1. Do you want me to create a separate issue instead where I describe the
    details of this bug?
  2. Do you have a policy for a branch name? Is the branch name atropos-tempfile
    fine or do you want to use the (prospective) ticket ID?
  3. I think this should get a test case, do you want me to submit one as part of
    this PR?

Further questions:

  1. Why is the type i string? Shouldn't it be
    Union[str, Union[list[ANTsImage, tuple[ANTsImage]]?
  2. Shouldn't atually accept any Iterable[ANTsImage container?
  3. The format string code in general is pretty hard to read and prone to error,
    as this bug shows. Would you accept a patch that refactors this to make it
    more pythonic and readable?

@ntustison
Copy link
Member

This is great. Thanks. I'll wait until the checks complete and then merge. If you want to contribute what is discussed in your questions, that would also be welcome. We're not as formal with pull request protocol as some other packages so this format is fine.

@ntustison ntustison merged commit 00675f4 into ANTsX:master Sep 14, 2022
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.

None yet

2 participants