[Mono-list] [PATCH] robust mono-handle-d?

[Dennis Haney]

>>>>> "Dick" == Dick Porter <dick@ximian.com> writes:

Dick> On Fri, Jun 07, 2002 at 04:35:52PM +0200, Dennis Haney wrote:
Dick> Not just file handles, there are many sorts.  Please change this
Dick> wording throughout the doc and the comments.
>>  I believe that was the only place I was wrong. All other places I
>> talk about filehandles I refer to the filehandle of the
>> client-sockets...

Dick> They're called "file descriptors" in Linux :)

heh, thats the word(s) I was looking for all along ;) Shall I change
it?  If so it will be after the commit...

>>  I just have one question about the daemon... Why does it exist?
>> Isn't it better performancewise[2] to just protect the shared area
>> with a mutex when allocation a new handle/shared mem segment or
>> changing refcnt? It will however be a less resilient to clients
>> that crash (the deamon cleans up ref'd handles if socket closes)

Dick> It's precisely because with a mutex the shared memory segment
Dick> can be left in a locked state. Also, it's not so easy to clean
Dick> up shared memory without it (you can't just mark it deleted when
Dick> creating it, because you can't attach any more readers to the
Dick> same segment after that).

Dick> I did some minimal performance testing, and I don't think the
Dick> daemon is particularly slow.

This should be added to the doc ;) I'll send a patch when the rest has
been committed...

>> Index: daemon.c @@ -120,9 +160,7 @@ }
>> 
>> if(_wapi_shared_data->handles[handle].ref==0) { - if
>> (open_handles[handle]!=0) { - g_warning (G_GNUC_PRETTY_FUNCTION ":
>> per-process open_handles mismatch, set to %d, should be 0",
>> open_handles[handle]); - } + g_assert(open_handles[handle] == 0);

Dick> This change shouldnt go in.  The daemon shouldn't quit if it's
Dick> still servicing clients, and it should clean up when it does
Dick> quit.

assert does not quit (and never has in any implementation I've seen),
g_assert is implemented as:

#define g_assert(expr)                  G_STMT_START{           \
     if (expr) { } else                                         \
       g_log (G_LOG_DOMAIN,                                     \
              G_LOG_LEVEL_ERROR,                                \
              "file %s: line %d (%s): assertion failed: (%s)",  \
              __FILE__,                                         \
              __LINE__,                                         \
              __PRETTY_FUNCTION__,                              \
              #expr);                   }G_STMT_END


Dick> If anything, it should just moan with a g_warning and set
Dick> open_handles[handle]=0;

>> @@ -314,9 +421,15 @@ process_new (idx, req.u.new.type); break; case
>> WapiHandleRequestType_Open: +#ifdef DEBUG +
>> g_assert(req.u.open.handle < _WAPI_MAX_HANDLES); +#endif

Dick> I'd prefer g_warning()s instead of g_assert()s here, even though
Dick> it's marked DEBUG.

See above

>> process_open (idx, req.u.open.handle); break; case
>> WapiHandleRequestType_Close: +#ifdef DEBUG +
>> g_assert(req.u.close.handle < _WAPI_MAX_HANDLES); +#endif

Dick> And here.

See above

>> process_close (idx, req.u.close.handle); break; case
>> WapiHandleRequestType_Scratch:


Dick> Everything else is fine to commit.

Dick> Thanks,

You're welcome

Dick> - Dick

-- 
Dennis
use Inline C => qq{void p(char*g){
printf("Just Another %s Hacker\n",g);}};p("Perl");