This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] cygwin_internal methods to get/set thread name


On Wed, 20 Dec 2017, Corinna Vinschen wrote:
Hi Mark,

A lot to discuss here.

Yes, but first let me say I'd call these "speculative" patches, things I found necessary during aio library development. I had an incorrect mental picture of the Cygwin DLL: I was treating it as just a user-space DLL where, for an add-on library, one was free to use Cygwin constructs or pthread constructs, or even Win32 constructs for that matter.

I've now updated that mental picture of the Cygwin DLL to treat it as a kernel, where one can only use constructs provided by Cygwin. So, in the aio library there will be cygthreads only and no pthreads or Win32 threads. (So I should also separately update the gmon profiler to use a cygthread rather than a Win32 thread ;-)).

So these patches reflect the earlier mental picture. Maybe none of the code makes sense in an accurate mental picture. I wanted to at least air the code to see if some use might be made of it before discarding it.

I think most if not all of this stuff wouldn't have been needed if I had started with cygthreads at the beginning of aio library development, but I was testing a lot of different scenarios and didn't want to have to build a Cygwin DLL each time.

First of all, can you please describe the scenario in which you'd want
to give a cygthread another name?  Why is the one given at
cygthread::create time not sufficient?

I wasn't thinking about cygthreads. I did the pthread-related methods first, then noticed the ENOSYS error being done for setting a cygthread name. It was easy to supply a ::setname method. It didn't occur to me that all the cygthread creation methods allow specification of a name.

Also, why should we need another, non-standard way to read/write a
pthread name, other than pthread_getname_np/pthread_setname_np?  What is
that supposed to accomplish?  Is there really any real-world scenario
which you can't handle with the official entry points?

I was using strace and getting annoyed with it displaying "unknown thread 0x###" for my aio threads. At that point they were pthreads and not cygthreads. So that was the impetus for the name-getting additions. Name-setting, that's another story. I thought that perhaps a debugging app might want to tag threads of a subject process with names if they don't already have names. But I concede there is no such app at present.

We really don't want to add more non-standard entry points than
absolutely necessary.  There are too many already, partially for
historic reasons.

I can easily agree with this. If my original use of pthreads for a core Cygwin service was the wrong way to go (I think it was) and we would rather not encourage that kind of thing, then the pthread-related methods should not be implemented. The cygthread name-setting method can go away too.

I do think the "courtesy" added code on the cygthread name-getting method has a use: stracing a user program whose pthreads are making Cygwin syscalls. The code in this block allows to get the user-supplied pthread name for use in strace logging, rather than having "unknown thread 0x###"
displayed.

On Dec 20 00:08, Mark Geisert wrote:
Add support to cygwin_internal() for setting a cygthread name and getting or setting a pthread name.  Also add support for getting the internal i/o handle for a given file descriptor.

Can you please break the log message in lines <= 72 chars?

Yes; git newbie error. I belatedly discovered 'git commit --amend' so I can add a proofreading step between 'git format-patch' and 'git send-email'.


@@ -710,6 +743,14 @@ cygwin_internal (cygwin_getinfo_types t, ...)
 	}
 	break;

+      case CW_GET_IO_HANDLE_FROM_FD:
+	{
+	  int fd = va_arg(arg, int);
+	  fhandler_base *fh = cygheap->fdtab[fd];
+	  res = (uintptr_t) (fh->get_io_handle ());
+	}
+	break;
+
       default:
 	set_errno (ENOSYS);
     }

Nope, we won't do that.  The functionality is already available via
_get_osfhandle included via <io.h>.

Argh; My search for an existing facility wasn't wide enough.

Also, note that this is, and always will be a kludge.  There are
scenarios in which more than one handle is attached to a descriptor
(e.g., ptys) and the function will return only one.

Understood. For the testcase scenarios I was/am running, the limited functionality of _get_osfhandle() is sufficient.

Thanks for looking the code over. If there's anything left to resubmit after all the whittling to be done, I'll happily do that.
Regards,

..mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]