patchCERTI - Patches: patch #6893, avoid racy sleep to establish...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #6893: avoid racy sleep to establish connection to rtia

Submitter:  Mathias Fröhlich <frohlich>
Submitted:  Tue 11 Aug 2009 04:32:15 PM UTC
   
 
Category:  CERTI Priority:  5 - Normal
Status:  Ready For Test Privacy:  Public
Assigned to:  erk Open/Closed:  Closed

Jump to the original submission

Wed 21 Oct 2009 10:53:19 AM UTC, comment #7: 

Will be in 3.3.3

Eric NOULARD <erk>
Group administrator
Sat 19 Sep 2009 05:51:14 AM UTC, comment #6: 

Thanks!

Mathias Fröhlich <frohlich>
Mon 14 Sep 2009 08:53:08 PM UTC, comment #5: 

Nice work Mathias,

I've just reviewed your latest patch and commited it
to CVS HEAD.

Thus certi-norace-3.diff is merged in.

Eric NOULARD <erk>
Group administrator
Mon 14 Sep 2009 06:19:23 PM UTC, comment #4: 

Ok, back from vacation and having a chance to boot a win32 machine again :)

The attached new patch

  • uses getgenopt for the options.
  • adds the socketpair call even in the windows case.

  This means we can now even on windows choose between tcp or not
The rest of the patch is basically the same.

Concerning the the polymorphic implementation. I think that this is a good idea to have. But lets move step by step. The next time I get a bunch of time ...

Greetings

Mathias

(file #18716)

Mathias Fröhlich <frohlich>
Fri 14 Aug 2009 09:52:48 AM UTC, comment #3: 

Hi,

Thank you for digging concerning the socketpair.
Regarding RTIA_USES_TCP unfortunately we cannot
get rid of it now.

The reason is that we did introduce this when
prototyping RTI Java Binding support.
https://savannah.nongnu.org/task/?6905
(unfinished work).

See this thread of discussion on the ML:
http://lists.nongnu.org/archive/html/certi-devel/2008-07/msg00007.html
too.

The basic idea, if someone (may be us or not) want to add
java/python/ruby/perl/etc... HLA binding he may do it
by talking to the RTIA through a "classic" TCP socket,
be it on Unix or Windows, this is very portable.

On the "ease of use" and "performance" side we would rather
go for a socketpair+fork and/or shared memory+thread.

Conclusion, we cannot get rid of RTIA_USE_TCP *but*
we may improve source code readability, by using
"truly" polymorphic Socket class in
Communication/RTIA/Message/...

However I find it far beyond your initial patch proposal
and I do not ask you to do that.

May be you can "just" implement your socket pair idea
(linux & windows) and try to keep RTIA_USE_TCP "as-is".

I'll try to improve cleaner/polymorphic socket classes
later, may be during the work for SHM.

Concerning the SHM work, there is currently no branch,
Jean-Baptiste is working on it, and we may create the
branch as soon as he is ready to.
And I'm sure he will be glad to have some help.
I add JBC in CC here and ask him to start a tracker task
on the SHM work in order to invite interested people to
contribute.

Concerning the "the need of having paths and library paths set correct that rtia can start up"
this is another story.
Did you try the "USE_FULL_RPATH" OPTION of the CMakeLists.txt?
This may free you of the library path problem.
More info here:
http://www.cmake.org/Wiki/CMake_RPATH_handling#Always_full_RPATH

Eric NOULARD <erk>
Group administrator
Fri 14 Aug 2009 05:36:46 AM UTC, comment #2: 


Hi,

I will change the command line interface source file.

Regarding the socketpair emulation:
The idea behind the patch matches exactly what you found.

I was digging yesterday further into the win32 problem and found a document from microsoft that tells me how I can make the parent local socket handle inheritted to the child process. The lack of knowledge about that lead me to the current implementation.

Additional to that, can we then get rid of the RTIA_USES_TCP code paths? These additional cases make it much harder to follow that part of the code.

When I have feedback regarding the RTIA_USES_TCP topic I will change the implementation and resend.

And yes, having rtia in shared memory would be way better for many aspects - but much more work.
As I would like to have a smooth use experience without the need of having paths and library paths set correct that rtia can start up, this would be a great thing to have.
Is there a branch in cvs or is there something I can help for that? I would like to have an open source RTI that 'just works' ...

Greetings

Mathias

Mathias Fröhlich <frohlich>
Wed 12 Aug 2009 08:09:37 AM UTC, comment #1: 

Thank you for reposting here Mathias.
I'm currently reviewing your patch.

Like I said before it looks OK but the reverse
socket server/client case in the windows case
even I found it nice.

Here are some remarks about your current patch.

1) The features are OK

2) As stated at the beginning of the files
   RTIA_cmdline.[hc] are generated using:
    gengetopt -i RTIA.ggo
  
   We usually avoid modifying them 'manually' but
   rather modify the RTIA.ggo file.
   We store RTIA_cmdline.[hc] in CVS in order to
   avoid depending on installed gengetopt since
   those files do not change very often and should
   be very portable C source file.

3) Concerning the socketpair call
   May be the code would be clearer if we clearly know
   that we want to emulate socketpair on Windows?
   That is for the unix case we use socket pair
   and for the Windows pair we implement a socketpair
   call emulation.
   I did found one: http://cantrip.org/socketpair.c
   but there must be others
   (not done a Krugle search yet)
   or we may write our own.
   This way we would have more consistent unix/win32/64
   code.

Would you be willing to try to implement (or re-use)
a socketpair emulation on Windows?

If we try to re-use we should select a implementation
with appropriate license (new BSD or LGPL for example)
and put the re-used code in separate files, for
example: socketpair_win32.c/h.

You must know that one CERTI develeper (Jean-Baptiste CHAUDRON)
is beginning to work on multithreaded alternatives for CERTI
such that Federate and RTIA would reside in the same process
(which cause security problem too) this would be another way
to "solve" the startup-race.

Eric NOULARD <erk>
Group administrator
Tue 11 Aug 2009 04:32:15 PM UTC, original submission:  

Asl already posted onto the mailing list, but by request from Eric this is now here:

  • On UNIX, the default way to set up this connection is changed to use the

socketpair library call to establish the two way connection to rtia.

  • As a side effect of the above we do no longer need to care for cleaning up

the named pipe file on UNIX which occasionally was not cleaned up in my test
environment.

  • On win32 the connection is still done by a local tcp socket, but for this

case the role of the server and client is reversed to avoid the race
condition. The application is acting as the server and the rtia process
connects to the listening server socket in the application.

  • The TCP port number does no longer default to the process id which should

eliminate some possible failures due to process id's that are equal to already
occupied port numbers.

  • Signals are unblocked  when a problem appears in the fork/exec path when

starting up rtia.

  • Open files are closed in the rtia childs process past the fork.


I have tested the attached patch on fedora linux 32 and 64 bits as well as on
win32 and win64.
I have done compile tests on all our unix machines I have access to at work.
This includes at least AIX5, Solaris-8, an ancient IRIX variant and HP-
UX-11.11.
Note that on some of the architectures certi does not compile as a whole, but
the changed files do compile.
The patch should apply on todays cvs.

Mathias Fröhlich <frohlich>

 

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

Attached Files
file #18716:  certi-norace-3.diff added by frohlich (47KiB - text/x-patch)
file #18557:  certi-norace.diff added by frohlich (27KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by erk (Fro the SHM work)
  • -email is unavailable- added by erk (Posted a comment)
  • -email is unavailable- added by frohlich (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.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-10-21 erk Open/ClosedOpen Closed
    2009-09-14 erk StatusIn Progress Ready For Test
    2009-09-14 frohlich Attached File- Added certi-norace-3.diff, #18716
    2009-08-14 erk Carbon-Copy- Added jbchaudron
    2009-08-12 erk StatusNone In Progress
        Assigned toNone erk
    2009-08-11 frohlich Attached File- Added certi-norace.diff, #18557

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code