Back to the main page.

Bug 3068 - ft_warning crashes when stack contains > 63 character name

Status CLOSED FIXED
Reported 2016-02-12 10:07:00 +0100
Modified 2016-06-14 16:14:52 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P5 critical
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

Ian Andolina - 2016-02-12 10:07:12 +0100

I call ft_spikedensity from a GUI, and currently it causes a full hang of Matlab. Debugging through I found the problem was that ft_postamble calls ft_warning, which has a subfunction fieldnameFromStack which call fixname which calls ft_warning when the stack(end) has > 63chars: line 209 of ft_warning: ``` fname = horzcat(fixname(stack(end).name)); ``` uses the END of the stack trace and calls fixname. If the end of the stack is >63 characters: ```matlab ft_warning(sprintf('%s exceeds MATLAB''s maximum name length of 63 characters and has been truncated to %s',str,str(1:63))); ``` then it calls ft_warning and gets locked in a recursive loop ad infinitum. My stack end: ``` K>> stack(end) ans = file: '/Applications/MATLAB_R2015b.app/toolbox/matlab/graphics/+matlab/+graphics/+internal/+figfile/@FigFile/read.m' name: '@(hObject,eventdata)spikes_UI('AnalMenu_Callback',hObject,eventdata,guidata(hObject))' line: 0 ``` The evidence of the recursive stack: ``` K>> stack(1) ans = file: '/Users/ian/Code/fieldtrip/utilities/ft_warning.m' name: 'fieldnameFromStack' line: 197 K>> stack(2) ans = file: '/Users/ian/Code/fieldtrip/utilities/ft_warning.m' name: 'ft_warning' line: 143 K>> stack(3) ans = file: '/Users/ian/Code/fieldtrip/utilities/private/fixname.m' name: 'fixname' line: 51 K>> stack(4) ans = file: '/Users/ian/Code/fieldtrip/utilities/ft_warning.m' name: 'fieldnameFromStack' line: 209 K>> stack(5) ans = file: '/Users/ian/Code/fieldtrip/utilities/ft_warning.m' name: 'ft_warning' line: 143 K>> stack(6) ans = file: '/Users/ian/Code/fieldtrip/utilities/private/fixname.m' name: 'fixname' line: 51 ```


Robert Oostenveld - 2016-02-15 19:10:11 +0100

the only call to ft_warning that I can find in ft_postamble (and the scripts it includes) is at the end of ft_postamble_history, where % clear warnings from ft_default, so that they don't end up in the next cfg ft_warning('-clear'); I don't think that is causing the problem. But ft_warning calling fixname and vice versa can certainly cause a problem...


Robert Oostenveld - 2016-02-15 21:46:50 +0100

I think this would fix it https://github.com/fieldtrip/fieldtrip/pull/83 Could you have a look at https://github.com/robertoostenveld/fieldtrip/commit/83a51738666753da5f3cdbfc94e22803acb4f7cb and see whether you agree? If so, then I'll merge the change with the master.


Ian Andolina - 2016-02-16 02:48:20 +0100

Yes, it is postamble_history that is triggering this. Your patch hasn't fixed it, after merging your changes here is my stack from within fieldnameFromStack, we are still recursing K>> stack(:).name ans = fieldnameFromStack ans = ft_warning ans = fixname ans = fieldnameFromStack ans = ft_warning ans = fixname ans = fieldnameFromStack ans = ft_warning ans = fixname ans = fieldnameFromStack ans = ft_warning ans = fixname ans = fieldnameFromStack ans = ft_warning ans = ft_postamble_history ans = ft_postamble ans = ft_spikedensity ans = plotDensity ans = spikes ans = sDensityPlot_Callback ans = spikes_UI ans = @(hObject,eventdata)spikes_UI('sDensityPlot_Callback',hObject,eventdata,guidata(hObject)) It looks like the logic of any is wrong here, there shouldn't be a ~: if ~any(strcmp({stack.file}, 'ft_warning.m')) My temporary hack was just to remove the call to ft_warning from fixname which does work.


Ian Andolina - 2016-02-16 03:18:19 +0100

If I remove the ~ it also now doesn't recurse...


Robert Oostenveld - 2016-02-16 13:34:33 +0100

Ah, I now see where it goes wrong. You are right, the ~ should not be there. If there is any previous ft_warning in the stack, the function should return immediately. I will merge it into fieldtrip/master.


Robert Oostenveld - 2016-02-16 13:37:48 +0100

thanks for looking along with the fix.


Ian Andolina - 2016-02-17 02:08:42 +0100

Thanks for fixing Robert, great to see FT has moved over to Github for development, should make it easier to contribute patches (I have an old Plexon V2 pull request that probably needs rebasing, but I'm in grant writing mode so not much time to code...)


Robert Oostenveld - 2016-06-14 16:14:52 +0200

Hereby I am closing multiple bugs that have been resolved for some time now. If you don't agree to the resolution, please reopen.