bugGNU Octave - Bugs: bug #63714, uitable may show nan with data=zero

 
 

bug #63714: uitable may show nan with data=zero

Submitter:  Liang Tang <lt1234>
Submitted:  Thu 26 Jan 2023 09:04:31 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  lt1234 Open/Closed:  * Closed
Release:  * 7.1.0 Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  8.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 09 Feb 2023 08:41:58 PM UTC, comment #10: 

I assumed the Feb 3 overnight build contained the fix for this issue. 

I ran with Feb 3 overnight build as a test.  Right now I am beyond what I used to get code crashes.  Please close the report if others agree. 

The table editable warning no longer appears.  Thanks.

Liang Tang <lt1234>
Wed 08 Feb 2023 03:40:42 PM UTC, comment #9: 

Okay to close?

Rik <rik5>
Group administrator
Sun 29 Jan 2023 06:11:27 PM UTC, comment #8: 

"Still some warnings from TSAN."

Yeah, I think there are still many threading problems with the way the graphics objects are handled.

I don't see any reason not to make this change on stable, so I'll do that much at least.

There is another call to warning in Table.cc in the function qStringValueFor:


    else
      {
        std::stringstream warn_string;
        warn_string << "Unknown conversion for datatype " << val.class_name ()
                    << " to " << format
                    << " for value " << val.string_value (true);
        warning ("%s", warn_string.str ().c_str ());

        text = Utils::fromStdString (val.string_value (true));
      }


I don't understand this code well enough to see how that warning can be triggered, but ultimately the result appears to be that an attempt is made to convert an octave_value to a string.  Given that the call to warning is not safe at this point, it seems best to just eliminate it.

I pushed the following change on stable and merged with default:

http://hg.savannah.gnu.org/hgweb/octave/rev/abe72b9464d6

John W. Eaton <jwe>
Group administrator
Fri 27 Jan 2023 10:24:29 PM UTC, comment #7: 

With this diff i do not see crashes. Still some warnings from TSAN.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 27 Jan 2023 09:24:49 PM UTC, comment #6: 

Hi John,

I have been editing my file, without any source control.  Right now, the trigger is a uicontrol/radiobutton callback. In the callback, it does the get/delete/add to the uitable rowname and data field. 

h.load_model = uicontrol ('style', 'radiobutton',  ... 
'units', 'normalized', ...
'string', 'Load Model', ...  
'callback', @load_model, ...
'position', [0.1 0.8 0.1 0.04 ], ...
'backgroundcolor', [255, 255, 255]/256,  ...
'tag', 'load_model');

Is this OK to try the recommendation you have?

Liang Tang <lt1234>
Fri 27 Jan 2023 07:40:22 PM UTC, comment #5: 

With Octave build from the current default branch on my debian system, I don't see NaN appearing in the table but I do see the "Table data is not editable at this location" warnings and I am able to cause a crash when running with the thread sanitizer.

If you add the property


'celleditcallback', @(h, event) disp (event)


to the uitable definition, is the crash avoided?  Or, with the following change to Octave, is the crash avoided?  At least for me, the warning was coming from Table::sendCellEditCallback and it looks like the problem is that Octave's warning function should not be called directly from the GUI thread.


diff --git a/libgui/graphics/Table.cc b/libgui/graphics/Table.cc
--- a/libgui/graphics/Table.cc
+++ b/libgui/graphics/Table.cc
@@ -558,8 +558,6 @@ OCTAVE_BEGIN_NAMESPACE(octave)
         emit gh_callback_event (m_handle, "celleditcallback",
                                 cellEditCallbackEventObject);
       }
-    else if (error.string_value ().length () > 0)
-      warning ("%s", error.string_value ().c_str ());
   }
 i
   void


To be compatible with Matlab, I don't think we should be issuing a warning if celleditcallback is not provided.

Also, it appears that this function is being called from the Table::itemChanged function when the indices of the edited item are out of range for the current data in the uitable.  I'm still trying to understand how that can happen.

John W. Eaton <jwe>
Group administrator
Fri 27 Jan 2023 02:27:50 PM UTC, comment #4: 

Here is another test on dev (9.0.0 hg id 2891372cce9f) version.
Also resulted in a crash. I wonder if crash is due to some resource limitation (stack or ram). Anyway the TSAN trace is attached.

Dmitri.
--


(file #54275)

Dmitri A. Sergatskov <dasergatskov>
Fri 27 Jan 2023 11:03:38 AM UTC, comment #3: 

Hi Dmitri,

I have been getting those Table data is not editable messages since uitable was released.  It didnot/donot make sense to me why a message was issued.  My guess has been that the table size increase has something to do with it.  However, on window version, I don't remember the info was presented the same as you have.  Your message listed findobj has something to do with it.  From what I experimented here few days ago, I tend to agree with this info.  I saw lots less issue when the findjob was replaced with the graphic handle directly in the set(...) call.

I cannot read the trace file and make any sense out of it.  Do you have any additional comment on it?  Thanks.

Liang Tang <lt1234>
Fri 27 Jan 2023 01:05:05 AM UTC, comment #2: 

I compiled octave (hg id 713de911848f) with -fsanitize=thread
and run this code. It crashed. The log is attached.

Dmitri.
--


(file #54269)

Dmitri A. Sergatskov <dasergatskov>
Fri 27 Jan 2023 12:20:41 AM UTC, comment #1: 

I can reproduce this problem with 7.2 (on linux). I got hard crash once and "nan"s in two-table version quite reproducibly.

I do not see those problems with current stable snapshot
(hg id 713de911848f). I do see intermittent warnings:


ii = 15
ii = 16
ii = 17
warning: Table data is not editable at this location.
warning: called from
    repmat at line 70 column 11
    circshift at line 108 column 7
    findobj at line 222 column 21
    show_nan at line 69 column 8

warning: Table data is not editable at this location.
warning: called from
    circshift at line 109 column 3
    findobj at line 222 column 21
    show_nan at line 69 column 8

warning: Table data is not editable at this location.
warning: called from
    findobj at line 227 column 5
    show_nan at line 69 column 8


that kind of suggests that there is still a race condition here.

If you want to try the same octave snapshot on Windows,
you should be able to get it from
 https://buildbot.octave.space/#/download
once it finished compiling.

Dmitri.
--




Dmitri A. Sergatskov <dasergatskov>
Thu 26 Jan 2023 09:04:31 PM UTC, original submission:  

I have a gui kept crashing when the tables and plots are being updated.  The gui contain a number of uitable, uicontrol and one axes.  There is no mex.  Everything to build the gui are in text form and exercising basically put contents in, delete contents and replace with other contents. When crash occurred, there are no true computation.  Only get/put/delete type of action. 

There is no consistency that I can detect for what causes the crash.  There are only numeric and character variables in uitable.  My prior issue is with logical variable.  There is no logical variable in this case. 
 
These  are what I have observed:   

(1) The same data contents can crash Octave but it may not crash Octave in a different occasion. 
(2) The same uitable celledit callback function can be exercised in the command window when the gui is up.  But I have never been able to reproduce the crash in command window using the identical callback m file. However, it is quite easy to crash with gui activation. 
(3) I tried by placing a number of save in the callback function.  The mat files saved before the crash do not show any abnormality. 
(4) The crashes typically occurred when one of the three uitable is being updated, not axes being updated.  So all the drawnow have zero impact on the crash.
(5) Placing pause into various locations of the callback reduces the likelihood of crash, but it does not eliminate the crash.  
(6) It is lot easier/quicker to crash in 7.1.0 than 6.3.0.    

From that I build an uitable only example.  Up to now, it did not introduce crash but it has some unexpected numbers (nan) show up while they should all be zero.  Please see the attached zip file.  There are two ofig that illustrate what I observe here. 

Thanks,

Liang Tang <lt1234>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #54275:  tsan_dev.txt.gz added by dasergatskov (12KiB - application/gzip)
file #54269:  tsan_crash_20230126.txt.gz added by dasergatskov (21KiB - application/gzip)
file #54268:  nan.zip added by lt1234 (10KiB - application/x-zip-compressed)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by lt1234 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-02-11 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2023-01-29 jwe Planned ReleaseNone 8.1.0
    2023-01-29 jwe StatusNone Ready For Test
    2023-01-27 dasergatskov Attached File- Added tsan_dev.txt.gz, #54275
    2023-01-27 dasergatskov Attached File- Added tsan_crash_20230126.txt.gz, #54269
    2023-01-26 lt1234 Attached File- Added nan.zip, #54268

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code