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

 
 

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

Submitted by:  None
Submitted on:  Sat 25 Aug 2018 02:05:06 PM UTC  
 
Category:  Libraries Severity:  3 - Normal
Priority:  3 - Low Item Group:  Other
Status:  Ready For Test Assigned to:  None
Originator Name:  Octave user Originator Email:  -email is unavailable-
Open/Closed:  Open Release:  4.4.1
Operating System:  Microsoft Windows

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( 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>
Project 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>
Project 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:

(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>
Project Administrator
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:
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:

With patch:

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 jwe (Posted a comment)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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.3