bugGNU Octave - Bugs: bug #54571, gethostname must not be called...

 
 

bug #54571: gethostname must not be called from within DllMain on Win32 systems

Submitter:  None
Submitted:  Sat 25 Aug 2018 02:05:06 PM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  3 - Low Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Octave user Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.4.1
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 12 Nov 2018 11:52:10 AM UTC, comment #6: 

Dear Mr Eaton,

Thank you for applying those changes. I was not able to test it because I can't run bootstrap. Did you build Octave with those changes? Is it possible to set a breakpoint at main and another one in gethostname? If the debugger hits the breakpoint at main first, than the problem should be fixed I think.

Anonymous
Thu 01 Nov 2018 03:10:36 AM UTC, comment #5: 

I pushed the following changeset for load-save:

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

and a similar one for oct-hist:

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

As I understand it, these changes should solve the problem of calling gethostname to initialize a global variable because there are no more global or file-scope static variables in those files.

John W. Eaton <jwe>
Group administrator
Wed 31 Oct 2018 04:58:31 PM UTC, comment #4: 

I would like to fix this, but not quite in the way of the proposed patch.  Instead, I'd prefer to create a class to manage the load/save functionality and have an instance of that owned by the interpreter, similar to the way we have been refactoring other instances of global variables.  As I understand the patch proposed here, we just trade several global variables for a global structure containing those same variables.  That change is separate from avoiding the call to gethostname before main is called.

John W. Eaton <jwe>
Group administrator
Sat 29 Sep 2018 10:11:40 AM UTC, comment #3: 

Dear Mr Miller,

The patch has been rewritten according to your suggestions. I hope it can be applied now.

(file #45117)

Anonymous
Thu 30 Aug 2018 07:28:34 AM UTC, comment #2: 

No, the delay occurs just because the dll is not initialized properly.

If I'm running the test in the attached file, it executes within 0.09 seconds:


$ time ./gethostname.exe
The Winsock 1.1 dll was found okay
hostname="SECNB123456"

real    0m0.091s
user    0m0.000s
sys     0m0.031s


(file #44899)

Anonymous
Wed 29 Aug 2018 07:07:54 PM UTC, comment #1: 

I have followed the links and I think I understand the point that this bug report is about. Basically on Windows, static initializers must not call functions that may be in shared libraries that haven't been fully loaded yet.

We could make some changes to defer calls to 'gethostname' until it is actually needed. But I don't think this patch is the right way to do that. If this is needed, we should properly rework those global string variables that depend on the host name to be API functions.

This patch is not ready to be applied.

How did you get a 20 second startup time from gethostname, is your system somehow configured that it doesn't even know its own name and has to reach another server, or timed out somehow querying the host name?

Mike Miller <mtmiller>
Group Member
Sat 25 Aug 2018 02:05:06 PM UTC, original submission:  

According to DllMain there several restrictions which functions may be called from within DllMain. Especially LoadLibrary must not be called directly nor indirectly Dynamic-Link Library Best Practices.

Although octave is not providing an implementation of DllMain, it will be generated by the compiler. Any initialized global or static variable of a Dll outside of a function will be initialized in DllMain.
For example the following definition in oct-hist.cc will actually call gethostname from within DllMain:

static std::string Vhistory_timestamp_format_string = default_history_timestamp_format ();

The behaviour of such a call could be undefined because it depends on other Dll's which might not be initialized properly if their DllMain has not been called before. So it might work or might not work depending on the load order of Dll's.

The patch in the attached file fixes this issue at least in case of gethostname. It simply delays the call to gethostname until main is called.

As a side effect this patch can reduce start-up time of octave depending on OS version and system configuration.

Without patch:

$ time /usr/local/bin/octave-cli -qf --eval x=pi
x =  3.1416

real    0m20.470s
user    0m0.000s
sys     0m0.031s


With patch:

$ time /usr/local/bin/octave-cli -qf --eval x=pi
x =  3.1416

real    0m0.485s
user    0m0.016s
sys     0m0.015s


Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #45117:  octave-gethostname2.patch added by None (11KiB - application/octet-stream)
file #44899:  gethostname.tar.bz2 added by None (1KiB - application/octet-stream)
file #44860:  octave-gethostname.patch added by None (2KiB - application/octet-stream)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

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

    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 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-12-13 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-11-01 jwe StatusPatch Submitted Ready For Test
    2018-10-30 mtmiller Dependencies- bugs #54912 is dependent
    2018-09-29 mtmiller Carbon-CopyRemoved 80942 -
    2018-09-29 mtmiller StatusPatch Reviewed Patch Submitted
    2018-09-29 None Attached File- Added octave-gethostname2.patch, #45117
    2018-08-30 None Attached File- Added gethostname.tar.bz2, #44899
    2018-08-29 mtmiller Priority5 - Normal 3 - Low
        StatusPatch Submitted Patch Reviewed
    2018-08-26 mtmiller Carbon-CopyRemoved 80942 -
    2018-08-26 mtmiller StatusNone Patch Submitted
    2018-08-25 None Attached File- Added octave-gethostname.patch, #44860

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code