make mono use global mutex instead of named mutex, like in CoreCLR #720

Merged
merged 1 commit into from Jun 30, 2016

Conversation

Projects
None yet
5 participants
- }
- }
+ ExecuteSynchronizedCoreCrossPlat(ioOperation);
+ return;

This comment has been minimized.

@drewgillies

drewgillies Jun 29, 2016

Contributor

Given that both cases in this if-else end in return, the returns can be removed altogether.

@drewgillies

drewgillies Jun 29, 2016

Contributor

Given that both cases in this if-else end in return, the returns can be removed altogether.

This comment has been minimized.

@rohit21agrawal

rohit21agrawal Jun 29, 2016

Contributor

fixed

@rohit21agrawal

rohit21agrawal Jun 29, 2016

Contributor

fixed

@drewgillies

This comment has been minimized.

Show comment
Hide comment
@drewgillies

drewgillies Jun 29, 2016

Contributor

Apart from above, :shipit: :)

Contributor

drewgillies commented Jun 29, 2016

Apart from above, :shipit: :)

}
+
+ return;

This comment has been minimized.

@alpaix

alpaix Jun 29, 2016

Contributor

not needed

@alpaix

alpaix Jun 29, 2016

Contributor

not needed

@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Jun 29, 2016

Contributor

Why do we want a global mutex rather than a named one all the time?

Contributor

yishaigalatzer commented Jun 29, 2016

Why do we want a global mutex rather than a named one all the time?

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jun 29, 2016

Contributor

@yishaigalatzer we can't ifdef the global mutex declaration because we can't detect if we are running against mono at buildtime.

Contributor

rohit21agrawal commented Jun 29, 2016

@yishaigalatzer we can't ifdef the global mutex declaration because we can't detect if we are running against mono at buildtime.

@rohit21agrawal rohit21agrawal changed the title from make mono use named mutex instead of global mutex, like in CoreCLR to make mono use global mutex instead of named mutex, like in CoreCLR Jun 29, 2016

@alpaix

This comment has been minimized.

Show comment
Hide comment
@alpaix

alpaix Jun 29, 2016

Contributor

@rohit21agrawal use Lazy global mutex?

Contributor

alpaix commented Jun 29, 2016

@rohit21agrawal use Lazy global mutex?

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jun 29, 2016

Contributor

@alpaix good idea. implemented.

Contributor

rohit21agrawal commented Jun 29, 2016

@alpaix good idea. implemented.

@alpaix

This comment has been minimized.

Show comment
Hide comment
@alpaix

alpaix Jun 29, 2016

Contributor

🆗 :shipit:

Contributor

alpaix commented Jun 29, 2016

🆗 :shipit:

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jun 29, 2016

Contributor

@yishaigalatzer @alpaix
Discussed the possibility of using ConcurrentUtilities.ExecuteWithFileLockedAsync in place of mutex here. However, since the method is asynchronous , it will take a huge amount of effort to plumb the async calls through all the methods. Therefore, decided to go with the mutex approach only.

Contributor

rohit21agrawal commented Jun 29, 2016

@yishaigalatzer @alpaix
Discussed the possibility of using ConcurrentUtilities.ExecuteWithFileLockedAsync in place of mutex here. However, since the method is asynchronous , it will take a huge amount of effort to plumb the async calls through all the methods. Therefore, decided to go with the mutex approach only.

@rohit21agrawal rohit21agrawal merged commit 584723b into dev Jun 30, 2016

@rohit21agrawal rohit21agrawal deleted the dev-ragrawal-monofix branch Jun 30, 2016

@rohit21agrawal rohit21agrawal restored the dev-ragrawal-monofix branch Jun 30, 2016

rohit21agrawal added a commit that referenced this pull request Jul 1, 2016

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jul 1, 2016

Contributor

The correct fix is being tracked through : #725

Contributor

rohit21agrawal commented Jul 1, 2016

The correct fix is being tracked through : #725

@rohit21agrawal rohit21agrawal deleted the dev-ragrawal-monofix branch Jul 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment