Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
Absurd Alhazred
Mar 27, 2010

by Athanatos
Is there a reason that someone would ever want a move constructor to be substantially different from a move assignment, or either be substantially different from a direct constructor with the same parameters as the original?

Adbot
ADBOT LOVES YOU

Xerophyte
Mar 17, 2008

This space intentionally left blank

Absurd Alhazred posted:

Is there a reason that someone would ever want a move constructor to be substantially different from a move assignment, or either be substantially different from a direct constructor with the same parameters as the original?

Sure. Take a std::vector.

foo = std::move(bar); would need to free whatever allocation foo had done to assignment, then take ownership of bar's buffer.
std::vector<int> foo(std::move(bar)); would never need to free any data, or even look at foo's state.
std::vector<int> bar(2097152, 0); std::vector<int> foo(2097152, 0); would do two allocations of 2 MB vectors, the moves would only need one. A move can also change way more of the internal object state than just constructing the object, much like the copy assign and copy construct.

In general, the language can't really make any assumptions on what sort of cleanup is needed for a move-assign -- maybe the class being moved to needs to release some network or disk resource, who knows? -- so you need to explicitly specify it by writing a move-assign operator.

Absurd Alhazred
Mar 27, 2010

by Athanatos

Xerophyte posted:

Sure. Take a std::vector.

foo = std::move(bar); would need to free whatever allocation foo had done to assignment, then take ownership of bar's buffer.
std::vector<int> foo(std::move(bar)); would never need to free any data, or even look at foo's state.
std::vector<int> bar(2097152, 0); std::vector<int> foo(2097152, 0); would do two allocations of 2 MB vectors, the moves would only need one. A move can also change way more of the internal object state than just constructing the object, much like the copy assign and copy construct.

In general, the language can't really make any assumptions on what sort of cleanup is needed for a move-assign -- maybe the class being moved to needs to release some network or disk resource, who knows? -- so you need to explicitly specify it by writing a move-assign operator.

The alternatives I was thinking of were more like:

C++ code:
std::vector<int> foo(200, 0);
vs
C++ code:
std::vector<int> bar(200, 0);
std::vector<int> foo;
foo = std::move(bar);
vs
C++ code:
std::vector<int> bar (200,0);
std::vector<int> foo(std::move(bar));
But yeah, I see your point, especially dealing with handles to resources.

I guess I was really thinking more like:
C++ code:
auto foo = std::vector<int>();
Which should be elided into
C++ code:
std::vector<int> foo;
or
C++ code:
std::vector<int> func()
{
 	std::vector<int>(30) result;
	for (auto i = 0U; i < result.size(); i++)
	{
		result[i] = i;
	}
	return result;
}

void main()
{
	auto foo = func();
}
which should be elided into:
C++ code:
std::vector<int>(30) foo;
for (auto i = 0U; i < foo.size(); i++)
{
	foo[i] = i;
}
Although now that I think of it, that requires the compiler to be allowed to optimize, as generally this would require going through the stack.

Anyway, I had some things mixed up in my head, thanks for the explanation! :)

xgalaxy
Jan 27, 2004
i write code
I'm not an atomics expert or anything so I'm curious. I was browsing UnrealEngine source and noticed they do stuff like:
code:
static FORCEINLINE int32 InterlockedExchange(volatile int32* Value, int32 Exchange)
{
    return __sync_lock_test_and_set(Value, Exchange);
}
But according to the GCC documentation you should always use __sync_lock_release afterwards which Unreal doesn't appear to be doing.
Is this a bug in their code or what am I missing?

Also what is the new __atomic_* version of this call? I'm mostly wondering about the memory model semantic.

For reference all of the platforms Unreal support follow Windows lead, meaning their Windows implementation uses the _Interlocked* family of functions.
Does that mean the __atomic_* versions would be using __ATOMIC_SEQ_CST?

Thanks.

Finding good information about this stuff seems rather difficult to google.

xgalaxy fucked around with this message at 06:46 on Apr 28, 2018

rjmccall
Sep 7, 2007

no worries friend
Fun Shoe

xgalaxy posted:

I'm not an atomics expert or anything so I'm curious. I was browsing UnrealEngine source and noticed they do stuff like:
code:
static FORCEINLINE int32 InterlockedExchange(volatile int32* Value, int32 Exchange)
{
    return __sync_lock_test_and_set(Value, Exchange);
}
But according to the GCC documentation you should always use __sync_lock_release afterwards which Unreal doesn't appear to be doing.

I don't know where you're getting that. There's nothing like that in the most recent (7.3) documentation.

If you were using atomics to implement a traditional mutex, yes, you would use __sync_lock_test_and_set (which is really grossly misnamed) to acquire the lock and __sync_lock_release to release it, but an atomic exchange can be useful for other things.

xgalaxy posted:

Also what is the new __atomic_* version of this call? I'm mostly wondering about the memory model semantic.

__atomic_exchange_n(Value, Exchange, __ATOMIC_ACQUIRE), or in C++ atomics, std::atomic_exchange_explicit(Value, Exchange, std::memory_order_acquire).

xgalaxy
Jan 27, 2004
i write code

rjmccall posted:

I don't know where you're getting that. There's nothing like that in the most recent (7.3) documentation.

If you were using atomics to implement a traditional mutex, yes, you would use __sync_lock_test_and_set (which is really grossly misnamed) to acquire the lock and __sync_lock_release to release it, but an atomic exchange can be useful for other things.


__atomic_exchange_n(Value, Exchange, __ATOMIC_ACQUIRE), or in C++ atomics, std::atomic_exchange_explicit(Value, Exchange, std::memory_order_acquire).

Thanks.

I think combined with the confusing name of the function with the wording of the documentation led me to believe that.

pseudorandom name
May 6, 2007

Windows specifies that InterlockedExchange is a full barrier, so that probably shouldn’t be using acquire.

xgalaxy
Jan 27, 2004
i write code

pseudorandom name posted:

Windows specifies that InterlockedExchange is a full barrier, so that probably shouldn’t be using acquire.

Ok. So is the equivalent __ATOMIC_SEQ_CST or __ATOMIC_ACQ_REL. I'm thinking the first like I originally thought...

What I'd really like is a better understanding of all of this. Anyone know of a good website or book that talks about these concepts in terms of the different apis available. Like Win32, the GCC/Clang compiler, and finally the std::atomics ?

I found C++ Concurrency in Action. It's a bit old but covers the C++ 11 thread and atomics stuff. But I doubt it will cover how they relate to Interlocked* equivalents for example.

xgalaxy fucked around with this message at 19:26 on Apr 28, 2018

SuperKlaus
Oct 20, 2005


Fun Shoe
Hi, my dumb rear end again. I need to use the accumulate function to multiply a set of integers and I need to use a binary function object. This is what I wrote:

#include <iostream>
#include <set>
#include <iterator>
#include <algorithm>
#include <numeric>
#include <functional>

using namespace std;

#include <cstdlib>
#include <ctime>

template<typename T>
class SumMultiplied{
public:
T operator()(const T& total, const T& value){
return total * value;
}
};

int main()
{
srand(time(NULL));
set<int, less <int>> intSet;
for(int i = 0; i < 10; ++i){
intSet.insert(rand() % 60 + 41);
}
ostream_iterator<int> output{cout, " "};
copy(intSet.begin(), intSet.end(), output);
int result = accumulate(intSet.begin(), intSet.end(), 1, SumMultiplied<int>{});
cout << result;
return 0;
}

It keeps returning wrong numbers. Negative numbers, half the time. What I don't understand is that if I change "total * value" to "total + value" so the binary function object sums up the set, it works perfectly. TBH I've basically cloned what my textbook has, but at least in CodeBlocks the multiply-accumulate isn't working. Yet when I literally clone the textbook example of summing the squares of integers, it is still wrong. What's my mistake? It's like there's something wrong with my *.

Jabor
Jul 16, 2010

#1 Loser at SpaceChem
Try printing out what numbers you're trying to multiply.

Jeffrey of YOSPOS
Dec 22, 2005

GET LOSE, YOU CAN'T COMPARE WITH MY POWERS
What is the expected value of your function?

SuperKlaus
Oct 20, 2005


Fun Shoe
Oh I print out the contents of the set so I can see them. That's how I know the addition is working. Assignment is to generate 10 randoms between 60 and 100 and multiply them though. I think I'll make it simpler, with a fixed bunch of numbers in an array or something, and make sure that works and report back.

Absurd Alhazred
Mar 27, 2010

by Athanatos
Try with a smaller set of numbers, too.

SuperKlaus
Oct 20, 2005


Fun Shoe
OH WAIT is it that the number is too big to fit in an int??? 10 integers between 60 and 100, too much. Dang! I never dealt with that before but it makes sense. Also because this works perfectly with smaller numbers like you suggested :kiddo:

Xarn
Jun 26, 2015
Now try std::cout << std::accumulate(intSet.begin(), intSet.end(), 1LL, std::multiplies<>{});

Sindai
Jan 24, 2007
i want to achieve immortality through not dying

SuperKlaus posted:

It keeps returning wrong numbers. Negative numbers, half the time.
"Suddenly I have a negative number where that shouldn't be possible" is something you'll probably see more than once in your programming career, and the first thing you should look for is a numeric overflow.

Try to keep in mind that the maximum value of a signed 32-bit integer is roughly two billion whenever you're using them.

SuperKlaus
Oct 20, 2005


Fun Shoe
You guys have been a big help. Typically when I see the unexpected negative numbers it's because I've done something like forgetting to initialize the element of an array that I used in the calculation. Max size of data types is that sort of thing ya learn in CS 101 and then never see in practice in your courses so you forget. How embarrassing.

leper khan
Dec 28, 2010
Honest to god thinks Half Life 2 is a bad game. But at least he likes Monster Hunter.

SuperKlaus posted:

You guys have been a big help. Typically when I see the unexpected negative numbers it's because I've done something like forgetting to initialize the element of an array that I used in the calculation. Max size of data types is that sort of thing ya learn in CS 101 and then never see in practice in your courses so you forget. How embarrassing.

I’ve run into type bound issues somewhat frequently so far in my career.

One of those things you just start testing to make sure things behave reasonably at the edges.

KoRMaK
Jul 31, 2012



It's been a long time since I did C++, and I'm jumping into an opengl project. Everything is ok, but I can't figure out this pointer issue I'm having.

1) I have a pointer, pointer_displayed_image, that is pointing to something.
2) I may want to change what it's pointing to later, so I stash it in another pointer, pointer_b
3) I want to use pointer_b to change what pointer_displayed_image is pointing at, so that things refering to pointer_displayed_image will use the newly loaded image.

I can't seem to do it the right way, I tried a double pointer to the first pointer but it doesn't seem to work

code:
char ** pointer_b = NULL;
char * pointer_displayed_image = LoadImage()
*pointer_b = pointer_displayed_image;
//... some stuff happens
char * pointer_new_image = LoadNewStuff();
*pointer_b = pointer_new_image ; //hopefully this will change what pointer_displayed_phrase points to?
I'm very wrong, but I don't know where.

KoRMaK fucked around with this message at 05:48 on May 19, 2018

Absurd Alhazred
Mar 27, 2010

by Athanatos

KoRMaK posted:

It's been a long time since I did C++, and I'm jumping into an opengl project. Everything is ok, but I can't figure out this pointer issue I'm having.

1) I have a pointer, pointer_displayed_image, that is pointing to something.
2) I may want to change what it's pointing to later, so I stash it in another pointer, pointer_b
3) I want to use pointer_b to change what pointer_displayed_image is pointing at, so that things refering to pointer_displayed_image will use the newly loaded image.

I can't seem to do it the right way, I tried a double pointer to the first pointer but it doesn't seem to work

code:
char ** pointer_b = NULL;
char * pointer_displayed_image = LoadImage()
*pointer_b = pointer_displayed_image;
//... some stuff happens
char * pointer_new_image = LoadNewStuff();
*pointer_b = pointer_new_image ; //hopefully this will change what pointer_displayed_phrase points to?
I'm very wrong, but I don't know where.

What you want is to have pointer_b point to the address of pointer_displayed_image, so:

C++ code:
char ** pointer_b = NULL;
char * pointer_displayed_image = LoadImage()
pointer_b = &pointer_displayed_image;
//... some stuff happens
char * pointer_new_image = LoadNewStuff();
*pointer_b = pointer_new_image ; //hopefully this will change what pointer_displayed_phrase points to?
Be sure that you actually want to have char* there. Honestly, I'd write it more like:

C++ code:
auto pointer_displayed_image = LoadImage();
auto pointer_b = &pointer_displayed_image;
//... some stuff happens
auto pointer_new_image = LoadNewStuff();
*pointer_b = pointer_new_image;
less code to change if the return type of LoadImage() changes or whatever. It's still a bit clunky, you ideally want to move away from using raw pointers in modern C++ unless you really have to.

Edit: for one thing, what happens to the resource pointer_displayed_image pointed to? Are you sure you're releasing it? It would make more sense to have some kind of smart pointer in pointer_displayed_image handling this, so you could just insert the new handle into it and destroy the old resource using the same operation, something like a custom assignment operator overload.

Absurd Alhazred fucked around with this message at 06:13 on May 19, 2018

KoRMaK
Jul 31, 2012



I tried that, but analogized it to my example and get this error

quote:

Severity Code Description Project File Line Suppression State
Error (active) a value of type "Renderer::Texture **" cannot be assigned to an entity of type "Renderer::Texture *" Bonzomatic c:\Sites\Bonzomatic\src\main.cpp 215

But, when I do the same code in using char pointers as described above, it works. Wth

C++ code:
Renderer::Texture ** _prev_frame_tex = NULL;
Renderer::Texture * tex = Renderer::CreateRGBA8TextureFromFile( fn );
*_prev_frame_tex = &tex; //compile error on this line
//...later
Renderer::Texture *_tex = Renderer::CreateA8TextureFromData(Renderer::nWidth, Renderer::nHeight, (unsigned char *)_buffer);
*_prev_frame_tex = _tex;

Absurd Alhazred
Mar 27, 2010

by Athanatos

KoRMaK posted:

I tried that, but analogized it to my example and get this error


But, when I do the same code in using char pointers as described above, it works. Wth

C++ code:
Renderer::Texture ** _prev_frame_tex = NULL;
Renderer::Texture * tex = Renderer::CreateRGBA8TextureFromFile( fn );
*_prev_frame_tex = &tex; //compile error on this line
//...later
Renderer::Texture *_tex = Renderer::CreateA8TextureFromData(Renderer::nWidth, Renderer::nHeight, (unsigned char *)_buffer);
*_prev_frame_tex = _tex;

Okay, first you want _prev_frame_tex = &tex, and second, you really need to somehow free the resource pointed to by tex before you assign anything else to it. And for your own sake give these variables more comprehensible names instead of using underscores to distinguish them.

Ralith
Jan 12, 2011

I see a ship in the harbor
I can and shall obey
But if it wasn't for your misfortune
I'd be a heavenly person today
For your own sake, learn about unique_ptr and RAII.

Zopotantor
Feb 24, 2013

...und ist er drin dann lassen wir ihn niemals wieder raus...

KoRMaK posted:

code:
char ** pointer_b = NULL;
char * pointer_displayed_image = LoadImage()
*pointer_b = pointer_displayed_image;

You're assigning through a null pointer here, for starters.

rjmccall
Sep 7, 2007

no worries friend
Fun Shoe
Yeah, the main thing here is that you're making really elementary mistakes with pointers. You see this a lot with newcomers to the language who are basically panicking and trying to apply everything they know about pointers at once.

A pointer is just a value, like an int or a float, and you can assign it around just like you assign around an int or a float. Having a variable of pointer type is not special in this sense; if you want to change the value of the variable to point to something else, you just assign it another pointer value without using the * operator or anything. The * operator is specifically for when you want to read from or write to the memory that the pointer is pointing at; it doesn't change the pointer value itself. Seriously, you need to go practice with pointers completely independently from OpenGL for a bit.

Once you've gotten the hang of that, hopefully you'll see that what you're trying to do is not really possible. If you've got a variable set to the number 5, you can copy that value to a bunch of places, but now you've just got a bunch of different places that store the number 5, and there's no way to automatically change them all to store the number 6 just by doing something to the original variable. The same thing is true with pointers — you can copy the pointer value out of pointer_displayed_image to a bunch of places, but that just means you have a bunch of places independently pointing at the same memory; there's nothing you can do with pointer_displayed_image that'll change any of them.

Now, you could instead copy the address of pointer_displayed_image to all of those places. Then you can just assign a new pointer into it, and they'll see the new value when they load it. But if you do that, you'll have to be really careful about the lifetime of pointer_displayed_image — as soon as it goes out of scope, all those places you've stored its address will be in big trouble.

Also, everyone else is right and you probably shouldn't be messing around with raw pointers, once you've taught yourself how they work again.

Absurd Alhazred
Mar 27, 2010

by Athanatos
Yeah, the best way to think about it is in terms of how to manage the lifetime of the resource you're allocating. Then you find a combination of std::unique_ptr's, std::shared_ptr's, and potentially custom wrapper classes to help you perform this management.

Mooey Cow
Jan 27, 2018

by Jeffrey of YOSPOS
Pillbug

Ralith posted:

For your own sake, learn about unique_ptr and RAII.

This is the correct answer. If you haven't done C++ in a long while, you will want to go back and relearn what the language is today, because C-style pointers galore should be extremely rare.

You say you want to change the pointer to a resource used by other parts of your code, but this is almost certainly not what you want to do and seems very dangerous. You probably want to change the actual resource instead, like loading new data into the texture, or something. If that seems like a heavy operation, make the texture swappable; this will be equivalent to your current code but safer, with the pointer shuffling kept localized to one place.
The whole thing seems like a potential ownership problem though, since all the other code must be aware that the resource can be changed from outside without notice (this will make it very hard to reason about what actually happens in your code, in turn making debugging a nightmare).


(Personal context: Just spent two weeks trying to figure out why a 2 million LOC 98-style "C++"-project made entirely from macros and global variables with not a destructor in sight, was crashing randomly (answer: gee golly it was a data race))

F_Shit_Fitzgerald
Feb 2, 2017



I'm working on an assignment, and for some reason, my code is leaking memory. I can't figure out why.

Just to make sure I'm doing things correctly: is this the proper way, in constructors, to initialize a dynamic array class member?

code:
MyClass::MyClass ()
 : numItems(1)
{
   dataArray = new int[numItems];
}
Also, if you have another constructor that takes an array as a parameter, is it proper to call delete on that passed array within the body of the constructor?

For instance, something like:
code:
MyClass::MyClass(int items, int * theArray)
 : numItems(items), dataArray(new int[items])
{
  ...
  delete [] theArray
}
Hopefully, these questions are general enough not to be seen as trying to get help in doing my work for me, which is not my intention. I'm just a bit confused about using dynamic arrays.

Mooey Cow
Jan 27, 2018

by Jeffrey of YOSPOS
Pillbug
Those things seem syntactically correct, but how do you know the second constructor actually gets an array passed in? How does the calling code know that it can't use the pointer it passed in afterwards (since it will be deleted)?


Maybe the assignment demands that code should work like that, but the correct ways of handling dynamic memory these days start with std:: unless you really know what you're doing.

Absurd Alhazred
Mar 27, 2010

by Athanatos

F_Shit_Fitzgerald posted:

I'm working on an assignment, and for some reason, my code is leaking memory. I can't figure out why.

Just to make sure I'm doing things correctly: is this the proper way, in constructors, to initialize a dynamic array class member?

code:
MyClass::MyClass ()
 : numItems(1)
{
   dataArray = new int[numItems];
}

Just make sure to call delete[] dataArray in the destructor (MyClass::~MyClass()).

quote:

Also, if you have another constructor that takes an array as a parameter, is it proper to call delete on that passed array within the body of the constructor?

For instance, something like:
code:
MyClass::MyClass(int items, int * theArray)
 : numItems(items), dataArray(new int[items])
{
  ...
  delete [] theArray
}

It's usually bad form to destroy a parameter for a constructor unless you're explicitly using move semantics, which is probably beyond where you are now. Does the person using MyClass know that you're going to be deleting theArray? What are you doing with theArray other than deleting it?

F_Shit_Fitzgerald
Feb 2, 2017



Absurd Alhazred posted:

It's usually bad form to destroy a parameter for a constructor unless you're explicitly using move semantics, which is probably beyond where you are now. Does the person using MyClass know that you're going to be deleting theArray? What are you doing with theArray other than deleting it?

This constructor places the elements from the passed array into the dynamic array class member. So it's something like this:

code:
MyArray::MyArray(int items, int * arr)
  : totalItems(items), dataAray(new int[items])
{
   for (int i = 0; i < totalItems; ++i) {
      dataArray[i] = arr[i];
   }

   delete [] arr;
}
What I was thinking here is that deleting the pointer to the passed array would free up memory, but I'm not completely sure that's 100% kosher.

e: There is a destructor for the class. It was one of the first things I coded because it was one of the easiest fixes to make.

F_Shit_Fitzgerald fucked around with this message at 18:47 on May 19, 2018

Absurd Alhazred
Mar 27, 2010

by Athanatos

F_Shit_Fitzgerald posted:

This constructor places the elements from the passed array into the dynamic array class member. So it's something like this:

code:
MyArray::MyArray(int items, int * arr)
  : totalItems(items), dataAray(new int[items])
{
   for (int i = 0; i < totalItems; ++i) {
      dataArray[i] = arr[i];
   }

   delete [] arr;
}
What I was thinking here is that deleting the pointer to the passed array would free up memory, but I'm not completely sure that's 100% kosher.

e: There is a destructor for the class. It was one of the first things I coded because it was one of the easiest fixes to make.

What you're passing along is just a pointer to an array. If that array that you are pointing to is then being used elsewhere, you are using after free, which is a big no-no. If it's not even pointing to allocated memory, that's a bigger no-no.

F_Shit_Fitzgerald
Feb 2, 2017



Absurd Alhazred posted:

What you're passing along is just a pointer to an array. If that array that you are pointing to is then being used elsewhere, you are using after free, which is a big no-no. If it's not even pointing to allocated memory, that's a bigger no-no.

I'll take a look at that. But the array that's passed is discarded. The class member is what's used in the operations this class does. It sounds like the delete is extraneous, so I'll just take it out.

Thanks for the help.

VikingofRock
Aug 24, 2008




I'm trying to write a C++ program (targeting UNIX) which launches multiple sub-processes from different threads, with a timeout parameter for each subprocess (after which the subprocess is killed). I think I've got a pretty good understanding of fork() and of the exec() family of functions, but where I'm a little lost is in handling the timeout and in reaping the child process. My original plan was to call sigtimedwait() in a loop to wait for a given subprocess to send a SIGCHLD, and then to reap the child with waitpid(). But I think this doesn't work in a multi-threaded environment: as far as I can tell, SIGCHLD gets sent to an arbitrary thread, so there's no guarantee that the thread which launches a subprocess will ever see the corresponding SIGCHLD from its termination. So what's the best practice here?

Dren
Jan 5, 2001

Pillbug

VikingofRock posted:

I'm trying to write a C++ program (targeting UNIX) which launches multiple sub-processes from different threads, with a timeout parameter for each subprocess (after which the subprocess is killed). I think I've got a pretty good understanding of fork() and of the exec() family of functions, but where I'm a little lost is in handling the timeout and in reaping the child process. My original plan was to call sigtimedwait() in a loop to wait for a given subprocess to send a SIGCHLD, and then to reap the child with waitpid(). But I think this doesn't work in a multi-threaded environment: as far as I can tell, SIGCHLD gets sent to an arbitrary thread, so there's no guarantee that the thread which launches a subprocess will ever see the corresponding SIGCHLD from its termination. So what's the best practice here?

My general approach to the problem that signals can arrive on any thread is to use pthread_sigmask to block all signals from all but one of my threads. Then, in that one thread, I do whatever needs to happen to respond to the signal.

In your case the thread that receives signals could have a shared (threadsafe) datastructure with other threads where it holds a condition variable, the pid they started, and an std::atomic_bool. It would do a sigsuspend to wait for SIGCHLD then call wait(), look up the cv associated with the pid that wait() returns, set a shared std::atomic_bool and notify the thread that started the subprocess. In the thread that started the subprocess you would have just been waiting on the condition variable with wait_for until the timeout.

My larger question is do you need threads for what you're doing at all? But that's something for you to think about.

Dren fucked around with this message at 01:41 on May 27, 2018

VikingofRock
Aug 24, 2008




Dren posted:

My general approach to the problem that signals can arrive on any thread is to use pthread_sigmask to block all signals from all but one of my threads. Then, in that one thread, I do whatever needs to happen to respond to the signal.

In your case the thread that receives signals could have a shared (threadsafe) datastructure with other threads where it holds a condition variable, the pid they started, and an std::atomic_bool. It would do a sigsuspend to wait for SIGCHLD then call wait(), look up the cv associated with the pid that wait() returns, set a shared std::atomic_bool and notify the thread that started the subprocess. In the thread that started the subprocess you would have just been waiting on the condition variable with wait_for until the timeout.

My larger question is do you need threads for what you're doing at all? But that's something for you to think about.

This is part of a (much) larger project with threads baked in, and at this point it's pretty hard to take them out. Your solution is good, but I'm not sure I like the complication of having to call pthread_sigmask() in each thread. It would be nice to keep the sub-process launch handling all in one area of the code.

Maybe the simplest solution is to launch timeout <timeout> foo arg1 arg2 instead of foo arg1 arg2, and then just use waitpid() in the calling thread instead of messing with signals at all.

Dren
Jan 5, 2001

Pillbug
You call pthread_sigmask in the parent thread. Child threads inherit the parent thread’s sigmask. It’s not a bad idea to mask off all signals anyway to keep the code in the not-signal-handling threads from having to care about handling EINTR.

Dren fucked around with this message at 02:54 on May 27, 2018

pseudorandom name
May 6, 2007

Use SA_RESTART and don't deal with EINTR at all.

Dren
Jan 5, 2001

Pillbug
On the topic of threads does anyone know a safe way to cancel a pthread in c++? When I looked into it the answer seemed like no. The pthread implementation of thread cancellation is to throw an unnamed (though there is a gnu extension that gives it a name) exception at a deferred cancellation point. The deferred cancellation points are pretty much every syscall so this is a huge problem if you have a destructor with a call to close() since you could be in the destructor because of stack unwinding from a previous exception when you hit the deferred cancellation point and throw again, terminating your program.

Adbot
ADBOT LOVES YOU

rjmccall
Sep 7, 2007

no worries friend
Fun Shoe
Asynchronously forcing the tear-down of another thread is just a deeply terrible idea. It really needs the entire programming language to be designed around it, e.g. by doing something FRP-ish so that the vast majority of your program can be side-effect-free and there's only a small critical section that actually needs non-trivial logic to be safe against cancellation. Assuming your environment is less constrained, you should just forget about pthread_cancel and create your own safe-points to check for aborts.

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply